workqueue: Add a decrement-after-return and wake if 0 facility

RFC PATCH In review David Howells 5 participants 26 messages Sep 1, 2017 – Nov 9, 2017

Subsystems (18)

DRM drivers Media (V4L/DVB) VFS DRM misc +14 more subsystems

lore

Share & tools

lore

David Howells

author maintainer

2017-09-01 15:40 UTC #0 lore raw

RFC PATCH 1/11 workqueue: Add a decrement-after-return and wake if 0 facility

Add a facility to the workqueue subsystem whereby an atomic_t can be registered by a work function such that the work function dispatcher will decrement the atomic after the work function has returned and then call wake_up_atomic() on it if it reached 0.

This is analogous to complete_and_exit() for kernel threads and is used to avoid a race between notifying that a work item is about to finish and the .text segment from a module being discarded.

The way this is used is that the work function calls:

	dec_after_work(atomic_t *counter);

to register the counter and then process_one_work() calls it, potentially wakes it and clears the registration.

The reason I've used an atomic_t rather than a completion is that (1) it takes up less space and (2) it can monitor multiple objects.

Signed-off-by David Howells <[email protected]>

cc Tejun Heo <[email protected]>

cc Lai Jiangshan <[email protected]>

---

 include/linux/workqueue.h   |    1 +
 kernel/workqueue.c          |   25 +++++++++++++++++++++++++
 kernel/workqueue_internal.h |    1 +
 3 files changed, 27 insertions(+)

include/linux/workqueue.h +1 -0

index db6dc9dc0482..ceaed1387e9b 100644
@@ -451,6 +451,7 @@ extern bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
 
 extern void flush_workqueue(struct workqueue_struct *wq);
 extern void drain_workqueue(struct workqueue_struct *wq);
+extern void dec_after_work(atomic_t *counter);
 
 extern int schedule_on_each_cpu(work_func_t func);

kernel/workqueue.c +25 -0

index ca937b0c3a96..2936ad0ab293 100644
@@ -2112,6 +2112,12 @@ __acquires(&pool->lock)
 		dump_stack();
 	}
 
+	if (worker->dec_after) {
+		if (atomic_dec_and_test(worker->dec_after))
+			wake_up_atomic_t(worker->dec_after);
+		worker->dec_after = NULL;
+	}
+
 	/*
 	 * The following prevents a kworker from hogging CPU on !PREEMPT
 	 * kernels, where a requeueing work item waiting for something to

@@ -3087,6 +3093,25 @@ int schedule_on_each_cpu(work_func_t func)
 }
 
 /**
+ * dec_after_work - Register counter to dec and wake after work func returns
+ * @counter: The counter to decrement and wake
+ *
+ * Register an atomic counter to be decremented after a work function returns
+ * to the core.  The counter is 'woken' if it is decremented to 0.  This allows
+ * synchronisation to be effected by one or more work functions in a module
+ * without leaving a window in which the work function code can be unloaded.
+ */
+void dec_after_work(atomic_t *counter)
+{
+	struct worker *worker = current_wq_worker();
+
+	BUG_ON(!worker);
+	BUG_ON(worker->dec_after);
+	worker->dec_after = counter;
+}
+EXPORT_SYMBOL(dec_after_work);
+
+/**
  * execute_in_process_context - reliably execute the routine with user context
  * @fn:		the function to execute
  * @ew:		guaranteed storage for the execute work structure (must

kernel/workqueue_internal.h +1 -0

index 8635417c587b..94ea1ca9b01f 100644
@@ -28,6 +28,7 @@ struct worker {
 
 	struct work_struct	*current_work;	/* L: work being processed */
 	work_func_t		current_func;	/* L: current_work's fn */
+	atomic_t		*dec_after;	/* Decrement after func returns */
 	struct pool_workqueue	*current_pwq; /* L: current_work's pwq */
 	bool			desc_valid;	/* ->desc is valid */
 	struct list_head	scheduled;	/* L: scheduled works */

David Howells

author maintainer

2017-09-01 15:41 UTC #1 lore raw

RFC PATCH 2/11 refcount: Implement inc/decrement-and-return functions

Implement functions that increment or decrement a refcount_t object and return the value. The dec-and-ret function can be used to maintain a counter in a cache where 1 means the object is unused, but available and the garbage collector can use refcount_dec_if_one() to make the object unavailable. Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

The interface is as follows:

	unsigned int refcount_dec_return(refcount_t *r);
	unsigned int refcount_inc_return(refcount_t *r);

instead.

Signed-off-by David Howells <[email protected]>

cc Peter Zijlstra <[email protected]>

cc Kees Cook <[email protected]>

---

 include/linux/refcount.h |   12 ++++++++
 lib/refcount.c           |   67 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

include/linux/refcount.h +12 -0

index 591792c8e5b0..566c0cea7343 100644
@@ -52,6 +52,8 @@ extern __must_check bool refcount_sub_and_test(unsigned int i, refcount_t *r);
 
 extern __must_check bool refcount_dec_and_test(refcount_t *r);
 extern void refcount_dec(refcount_t *r);
+extern __must_check unsigned int refcount_inc_return(refcount_t *r);
+extern __must_check unsigned int refcount_dec_return(refcount_t *r);
 #else
 static inline __must_check bool refcount_add_not_zero(unsigned int i, refcount_t *r)
 {

@@ -87,6 +89,16 @@ static inline void refcount_dec(refcount_t *r)
 {
 	atomic_dec(&r->refs);
 }
+
+static inline unsigned int refcount_inc_return(refcount_t *r)
+{
+	return atomic_inc_return(&r->refs);
+}
+
+static inline unsigned int refcount_dec_return(refcount_t *r)
+{
+	return atomic_dec_return(&r->refs);
+}
 #endif /* CONFIG_REFCOUNT_FULL */
 
 extern __must_check bool refcount_dec_if_one(refcount_t *r);

lib/refcount.c +67 -0

79 lines not on this page · raw message

Peter Zijlstra

maintainer

2017-09-01 16:42 UTC #2 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 8 lines · David Howells

Implement functions that increment or decrement a refcount_t object and return the value. The dec-and-ret function can be used to maintain a counter in a cache where 1 means the object is unused, but available and the garbage collector can use refcount_dec_if_one() to make the object unavailable. Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

Quoted 5 lines

The interface is as follows:

	unsigned int refcount_dec_return(refcount_t *r);
	unsigned int refcount_inc_return(refcount_t *r);

I'm not immediately seeing how wanting 1 to mean unused leads to requiring these two functions.

If you'll remember, I did that for inode_count and only needed dec_unless().

David Howells

author maintainer

2017-09-01 21:15 UTC #3 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines
	unsigned int refcount_dec_return(refcount_t *r);
	unsigned int refcount_inc_return(refcount_t *r);
I'm not immediately seeing how wanting 1 to mean unused leads to requiring these two functions.

Did you read the other other part of the description?

Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

Quoted 2 lines

If you'll remember, I did that for inode_count and only needed dec_unless().

I don't remember. inode_count? I can't find such a thing - did you mean i_count? I don't find anything matching "dec_unless.*i_count" either.

David

Peter Zijlstra

maintainer

2017-09-01 21:50 UTC #4 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 6 lines · David Howells

Quoted 4 lines · Peter Zijlstra
Quoted 3 lines
	unsigned int refcount_dec_return(refcount_t *r);
	unsigned int refcount_inc_return(refcount_t *r);

Quoted 8 lines

Quoted 2 lines
I'm not immediately seeing how wanting 1 to mean unused leads to requiring these two functions.
Did you read the other other part of the description?
Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

I must admit to having overlooked that. But can we treat the two issues separately? They are quite distinct.

Quoted 5 lines

Quoted 2 lines
If you'll remember, I did that for inode_count and only needed dec_unless().
I don't remember. inode_count? I can't find such a thing - did you mean i_count? I don't find anything matching "dec_unless.*i_count" either.

Ah, yes, i_count. See these:

https://lkml.kernel.org/r/[email protected]
https://lkml.kernel.org/r/[email protected]

But looking at them, i_count was rather special, a normal GC based scheme doesn't need anything new AFAICT:

add:

	spin_lock(&map->lock)
	refcount_set(&obj->refs, 1);
	map_link(map, obj);
	spin_unlock(&map->lock);

lookup:

	rcu_read_lock();
	obj = map_find(map, key);
	if (obj && !refcount_inc_not_zero(&obj->refs))
	  obj = NULL;
	rcu_read_unlock();

	if (obj) {
	  /* use obj */
	  refcount_dec(&obj->refs); /* should never hit 0 */
	}

GC:

	spin_lock(&map->lock);
	map_for_each_obj_safe(obj, map) {
	  if (refcount_dec_if_one(&obj->refs)) {
	    map_unlink(map, obj);
	    rcu_free(obj);
	  }
	}
	spin_unlock(&map->lock);

Peter Zijlstra

maintainer

2017-09-01 22:03 UTC #5 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 8 lines · Peter Zijlstra

Quoted 5 lines
Did you read the other other part of the description?
Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).
I must admit to having overlooked that. But can we treat the two issues separately? They are quite distinct.

So for tracing purposes inc_return/dec_return don't cover the full set.

In particular: inc_not_zero, dec_not_one and dec_and_*lock are not covered.

dec_if_one I suppose we only care about the success case, in which case we knew it was one by inference.

David Howells

author maintainer

2017-09-01 22:51 UTC #6 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

In reply to Peter Zijlstra

Quoted 4 lines · Peter Zijlstra

	if (obj) {
	  /* use obj */
	  refcount_dec(&obj->refs); /* should never hit 0 */
	}

You've missed a bit: We need to tell the gc to run when we reduce the refcount to 1:

	if (obj) {
		...
		if (refcount_dec_return(&obj->refs) == 1)
			schedule_gc();
	}

David

Peter Zijlstra

maintainer

2017-09-04 07:30 UTC #7 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 7 lines · David Howells

Quoted 4 lines · Peter Zijlstra

	if (obj) {
	  /* use obj */
	  refcount_dec(&obj->refs); /* should never hit 0 */
	}

Quoted 8 lines

You've missed a bit: We need to tell the gc to run when we reduce the refcount to 1:
	if (obj) {
		...
		if (refcount_dec_return(&obj->refs) == 1)
			schedule_gc();
	}

Ah, so that isn't fundamental to having a GC. But yes if that's your requirement, then this makes sense.

Please clarify in the Changelog.

Christoph Hellwig

2017-09-04 15:36 UTC #8 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

In reply to David Howells

Quoted 7 lines · David Howells

Implement functions that increment or decrement a refcount_t object and return the value. The dec-and-ret function can be used to maintain a counter in a cache where 1 means the object is unused, but available and the garbage collector can use refcount_dec_if_one() to make the object unavailable. Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

Please just use a different interface for that instead of overloading refcount_t. The main use case of that type is that it is so simple that it is hard to get wrong (and have additional checking if things go wrong)

David Howells

author maintainer

2017-09-04 16:08 UTC #9 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines
Implement functions that increment or decrement a refcount_t object and return the value. The dec-and-ret function can be used to maintain a counter in a cache where 1 means the object is unused, but available and the garbage collector can use refcount_dec_if_one() to make the object unavailable. Further, both functions can be used to accurately trace the refcount (refcount_inc() followed by refcount_read() can't be considered accurate).

Quoted 4 lines

Please just use a different interface for that instead of overloading refcount_t. The main use case of that type is that it is so simple that it is hard to get wrong (and have additional checking if things go wrong)

Which bit are you objecting to? Wanting to use a refcount_t with 1 to represent an otherwise-unreferenced object sat in a cache? Or wanting to display accurate usage counts when tracing gets and puts of objects?

David

Christoph Hellwig

2017-09-05 06:45 UTC #10 lore raw

Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

Quoted 3 lines · David Howells

Which bit are you objecting to? Wanting to use a refcount_t with 1 to represent an otherwise-unreferenced object sat in a cache? Or wanting to display accurate usage counts when tracing gets and puts of objects?

Primarily the first, but also any feature creap of the refcount_t in general.

David Howells

author maintainer

2017-09-01 15:41 UTC #11 lore raw

RFC PATCH 3/11 Pass mode to wait_on_atomic_t() action funcs and provide default actions

In reply to David Howells

Make wait_on_atomic_t() pass the TASK_* mode onto its action function as an extra argument and make it 'unsigned int throughout.

Also, consolidate a bunch of identical action functions into a default function that can do the appropriate thing for the mode.

Also, change the argument name in the bit_wait*() function declarations to reflect the fact that it's the mode and not the bit number.

Signed-off-by David Howells <[email protected]>

cc Ingo Molnar <[email protected]>

---

 arch/mips/kernel/traps.c                           |   14 +---------
 drivers/gpu/drm/drm_dp_aux_dev.c                   |    8 +-----
 drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c |   10 +------
 drivers/media/platform/qcom/venus/hfi.c            |    8 +-----
 fs/afs/rxrpc.c                                     |    8 +-----
 fs/btrfs/extent-tree.c                             |   27 ++------------------
 fs/fscache/cookie.c                                |    2 +
 fs/fscache/internal.h                              |    2 -
 fs/fscache/main.c                                  |    9 -------
 fs/nfs/inode.c                                     |    4 +--
 fs/nfs/internal.h                                  |    2 +
 fs/ocfs2/filecheck.c                               |    8 +-----
 include/linux/wait_bit.h                           |   15 +++++++----
 kernel/sched/wait_bit.c                            |   18 ++++++++++---
 14 files changed, 37 insertions(+), 98 deletions(-)

arch/mips/kernel/traps.c +1 ~~-13~~

index b68b4d0726d3..3b3589ff3ecb 100644
@@ -1235,18 +1235,6 @@ static int default_cu2_call(struct notifier_block *nfb, unsigned long action,
 	return NOTIFY_OK;
 }
 
-static int wait_on_fp_mode_switch(atomic_t *p)
-{
-	/*
-	 * The FP mode for this task is currently being switched. That may
-	 * involve modifications to the format of this tasks FP context which
-	 * make it unsafe to proceed with execution for the moment. Instead,
-	 * schedule some other task.
-	 */
-	schedule();
-	return 0;
-}
-
 static int enable_restore_fp_context(int msa)
 {
 	int err, was_fpu_owner, prior_msa;

@@ -1256,7 +1244,7 @@ static int enable_restore_fp_context(int msa)
 	 * complete before proceeding.
 	 */
 	wait_on_atomic_t(&current->mm->context.fp_mode_switching,
-			 wait_on_fp_mode_switch, TASK_KILLABLE);
+			 atomic_t_wait, TASK_KILLABLE);
 
 	if (!used_math()) {
 		/* First time FP context user. */

drivers/gpu/drm/drm_dp_aux_dev.c +1 -7

index d34e5096887a..053044201e31 100644
@@ -263,12 +263,6 @@ static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux)
 	return aux_dev;
 }
 
-static int auxdev_wait_atomic_t(atomic_t *p)
-{
-	schedule();
-	return 0;
-}
-
 void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
 {
 	struct drm_dp_aux_dev *aux_dev;

@@ -283,7 +277,7 @@ void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
 	mutex_unlock(&aux_idr_mutex);
 
 	atomic_dec(&aux_dev->usecount);
-	wait_on_atomic_t(&aux_dev->usecount, auxdev_wait_atomic_t,
+	wait_on_atomic_t(&aux_dev->usecount, atomic_t_wait,
 			 TASK_UNINTERRUPTIBLE);
 
 	minor = aux_dev->index;

drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c +2 -8

index 7276194c04f7..f4815b96db0f 100644
@@ -271,13 +271,7 @@ struct igt_wakeup {
 	u32 seqno;
 };
 
-static int wait_atomic(atomic_t *p)
-{
-	schedule();
-	return 0;
-}
-
-static int wait_atomic_timeout(atomic_t *p)
+static int wait_atomic_timeout(atomic_t *p, unsigned int mode)
 {
 	return schedule_timeout(10 * HZ) ? 0 : -ETIMEDOUT;
 }

@@ -348,7 +342,7 @@ static void igt_wake_all_sync(atomic_t *ready,
 	atomic_set(ready, 0);
 	wake_up_all(wq);
 
-	wait_on_atomic_t(set, wait_atomic, TASK_UNINTERRUPTIBLE);
+	wait_on_atomic_t(set, atomic_t_wait, TASK_UNINTERRUPTIBLE);
 	atomic_set(ready, count);
 	atomic_set(done, count);
 }

drivers/media/platform/qcom/venus/hfi.c +1 -7

index c09490876516..e374c7d1a618 100644
@@ -88,12 +88,6 @@ int hfi_core_init(struct venus_core *core)
 	return ret;
 }
 
-static int core_deinit_wait_atomic_t(atomic_t *p)
-{
-	schedule();
-	return 0;
-}
-
 int hfi_core_deinit(struct venus_core *core, bool blocking)
 {
 	int ret = 0, empty;

@@ -112,7 +106,7 @@ int hfi_core_deinit(struct venus_core *core, bool blocking)
 
 	if (!empty) {
 		mutex_unlock(&core->lock);
-		wait_on_atomic_t(&core->insts_count, core_deinit_wait_atomic_t,
+		wait_on_atomic_t(&core->insts_count, atomic_t_wait,
 				 TASK_UNINTERRUPTIBLE);
 		mutex_lock(&core->lock);
 	}

fs/afs/rxrpc.c +1 -7

index 0bf191f0dbaf..cc7f7b3369ab 100644
@@ -41,12 +41,6 @@ static void afs_charge_preallocation(struct work_struct *);
 
 static DECLARE_WORK(afs_charge_preallocation_work, afs_charge_preallocation);
 
-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

@@ -121,7 +115,7 @@ void afs_close_socket(void)
 	}
 
 	_debug("outstanding %u", atomic_read(&afs_outstanding_calls));
-	wait_on_atomic_t(&afs_outstanding_calls, afs_wait_atomic_t,
+	wait_on_atomic_t(&afs_outstanding_calls, atomic_t_wait,
 			 TASK_UNINTERRUPTIBLE);
 	_debug("no outstanding calls");

fs/btrfs/extent-tree.c +3 ~~-24~~

index e3b0b4196d3d..14c8437eb4fc 100644
@@ -3934,16 +3934,9 @@ void btrfs_dec_nocow_writers(struct btrfs_fs_info *fs_info, u64 bytenr)
 	btrfs_put_block_group(bg);
 }
 
-static int btrfs_wait_nocow_writers_atomic_t(atomic_t *a)
-{
-	schedule();
-	return 0;
-}
-
 void btrfs_wait_nocow_writers(struct btrfs_block_group_cache *bg)
 {
-	wait_on_atomic_t(&bg->nocow_writers,
-			 btrfs_wait_nocow_writers_atomic_t,
+	wait_on_atomic_t(&bg->nocow_writers, atomic_t_wait,
 			 TASK_UNINTERRUPTIBLE);
 }

@@ -6523,12 +6516,6 @@ void btrfs_dec_block_group_reservations(struct btrfs_fs_info *fs_info,
 	btrfs_put_block_group(bg);
 }
 
-static int btrfs_wait_bg_reservations_atomic_t(atomic_t *a)
-{
-	schedule();
-	return 0;
-}
-
 void btrfs_wait_block_group_reservations(struct btrfs_block_group_cache *bg)
 {
 	struct btrfs_space_info *space_info = bg->space_info;

@@ -6551,8 +6538,7 @@ void btrfs_wait_block_group_reservations(struct btrfs_block_group_cache *bg)
 	down_write(&space_info->groups_sem);
 	up_write(&space_info->groups_sem);
 
-	wait_on_atomic_t(&bg->reservations,
-			 btrfs_wait_bg_reservations_atomic_t,
+	wait_on_atomic_t(&bg->reservations, atomic_t_wait,
 			 TASK_UNINTERRUPTIBLE);
 }

@@ -11036,12 +11022,6 @@ int btrfs_start_write_no_snapshoting(struct btrfs_root *root)
 	return 1;
 }
 
-static int wait_snapshoting_atomic_t(atomic_t *a)
-{
-	schedule();
-	return 0;
-}
-
 void btrfs_wait_for_snapshot_creation(struct btrfs_root *root)
 {
 	while (true) {

@@ -11050,8 +11030,7 @@ void btrfs_wait_for_snapshot_creation(struct btrfs_root *root)
 		ret = btrfs_start_write_no_snapshoting(root);
 		if (ret)
 			break;
-		wait_on_atomic_t(&root->will_be_snapshoted,
-				 wait_snapshoting_atomic_t,
+		wait_on_atomic_t(&root->will_be_snapshoted, atomic_t_wait,
 				 TASK_UNINTERRUPTIBLE);
 	}
 }

fs/fscache/cookie.c +1 -1

index 40d61077bead..ff84258132bb 100644
@@ -558,7 +558,7 @@ void __fscache_disable_cookie(struct fscache_cookie *cookie, bool invalidate)
 	 * have completed.
 	 */
 	if (!atomic_dec_and_test(&cookie->n_active))
-		wait_on_atomic_t(&cookie->n_active, fscache_wait_atomic_t,
+		wait_on_atomic_t(&cookie->n_active, atomic_t_wait,
 				 TASK_UNINTERRUPTIBLE);
 
 	/* Make sure any pending writes are cancelled. */

fs/fscache/internal.h +0 -2

index 97ec45110957..0ff4b49a0037 100644
@@ -97,8 +97,6 @@ static inline bool fscache_object_congested(void)
 	return workqueue_congested(WORK_CPU_UNBOUND, fscache_object_wq);
 }
 
-extern int fscache_wait_atomic_t(atomic_t *);
-
 /*
  * object.c
  */

fs/fscache/main.c +0 -9

index b39d487ccfb0..249968dcbf5c 100644
@@ -195,12 +195,3 @@ static void __exit fscache_exit(void)
 }
 
 module_exit(fscache_exit);
-
-/*
- * wait_on_atomic_t() sleep function for uninterruptible waiting
- */
-int fscache_wait_atomic_t(atomic_t *p)
-{
-	schedule();
-	return 0;
-}

fs/nfs/inode.c +2 -2

index 109279d6d91b..8b0475b3d48e 100644
@@ -85,9 +85,9 @@ int nfs_wait_bit_killable(struct wait_bit_key *key, int mode)
 }
 EXPORT_SYMBOL_GPL(nfs_wait_bit_killable);
 
-int nfs_wait_atomic_killable(atomic_t *p)
+int nfs_wait_atomic_killable(atomic_t *p, unsigned int mode)
 {
-	return nfs_wait_killable(TASK_KILLABLE);
+	return nfs_wait_killable(mode);
 }
 
 /**

fs/nfs/internal.h +1 -1

index dc456416d2be..39998b94a3de 100644
@@ -388,7 +388,7 @@ extern void nfs_evict_inode(struct inode *);
 void nfs_zap_acl_cache(struct inode *inode);
 extern bool nfs_check_cache_invalid(struct inode *, unsigned long);
 extern int nfs_wait_bit_killable(struct wait_bit_key *key, int mode);
-extern int nfs_wait_atomic_killable(atomic_t *p);
+extern int nfs_wait_atomic_killable(atomic_t *p, unsigned int mode);
 
 /* super.c */
 extern const struct super_operations nfs_sops;

fs/ocfs2/filecheck.c +1 -7

index 2cabbcf2f28e..e87279e49ba3 100644
@@ -129,19 +129,13 @@ static struct kobj_attribute ocfs2_attr_filecheck_set =
 					ocfs2_filecheck_show,
 					ocfs2_filecheck_store);
 
-static int ocfs2_filecheck_sysfs_wait(atomic_t *p)
-{
-	schedule();
-	return 0;
-}
-
 static void
 ocfs2_filecheck_sysfs_free(struct ocfs2_filecheck_sysfs_entry *entry)
 {
 	struct ocfs2_filecheck_entry *p;
 
 	if (!atomic_dec_and_test(&entry->fs_count))
-		wait_on_atomic_t(&entry->fs_count, ocfs2_filecheck_sysfs_wait,
+		wait_on_atomic_t(&entry->fs_count, atomic_t_wait,
 				 TASK_UNINTERRUPTIBLE);
 
 	spin_lock(&entry->fs_fcheck->fc_lock);

include/linux/wait_bit.h +9 -6

index 12b26660d7e9..40b9dd5b5ba1 100644
@@ -25,6 +25,8 @@ struct wait_bit_queue_entry {
 	{ .flags = p, .bit_nr = WAIT_ATOMIC_T_BIT_NR, }
 
 typedef int wait_bit_action_f(struct wait_bit_key *key, int mode);
+typedef int wait_atomic_t_action_f(atomic_t *counter, unsigned int mode);
+
 void __wake_up_bit(struct wait_queue_head *wq_head, void *word, int bit);
 int __wait_on_bit(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry, wait_bit_action_f *action, unsigned int mode);
 int __wait_on_bit_lock(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry, wait_bit_action_f *action, unsigned int mode);

@@ -33,7 +35,7 @@ void wake_up_atomic_t(atomic_t *p);
 int out_of_line_wait_on_bit(void *word, int, wait_bit_action_f *action, unsigned int mode);
 int out_of_line_wait_on_bit_timeout(void *word, int, wait_bit_action_f *action, unsigned int mode, unsigned long timeout);
 int out_of_line_wait_on_bit_lock(void *word, int, wait_bit_action_f *action, unsigned int mode);
-int out_of_line_wait_on_atomic_t(atomic_t *p, int (*)(atomic_t *), unsigned int mode);
+int out_of_line_wait_on_atomic_t(atomic_t *p, wait_atomic_t_action_f action, unsigned int mode);
 struct wait_queue_head *bit_waitqueue(void *word, int bit);
 extern void __init wait_bit_init(void);

@@ -50,10 +52,11 @@ int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync
 		},								\
 	}
 
-extern int bit_wait(struct wait_bit_key *key, int bit);
-extern int bit_wait_io(struct wait_bit_key *key, int bit);
-extern int bit_wait_timeout(struct wait_bit_key *key, int bit);
-extern int bit_wait_io_timeout(struct wait_bit_key *key, int bit);
+extern int bit_wait(struct wait_bit_key *key, int mode);
+extern int bit_wait_io(struct wait_bit_key *key, int mode);
+extern int bit_wait_timeout(struct wait_bit_key *key, int mode);
+extern int bit_wait_io_timeout(struct wait_bit_key *key, int mode);
+extern int atomic_t_wait(atomic_t *counter, unsigned int mode);
 
 /**
  * wait_on_bit - wait for a bit to be cleared

@@ -250,7 +253,7 @@ wait_on_bit_lock_action(unsigned long *word, int bit, wait_bit_action_f *action,
  * outside of the target 'word'.
  */
 static inline
-int wait_on_atomic_t(atomic_t *val, int (*action)(atomic_t *), unsigned mode)
+int wait_on_atomic_t(atomic_t *val, wait_atomic_t_action_f action, unsigned mode)
 {
 	might_sleep();
 	if (atomic_read(val) == 0)

kernel/sched/wait_bit.c +14 -4

index f8159698aa4d..84cb3acd9260 100644
@@ -183,7 +183,7 @@ static int wake_atomic_t_function(struct wait_queue_entry *wq_entry, unsigned mo
  */
 static __sched
 int __wait_on_atomic_t(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_entry,
-		       int (*action)(atomic_t *), unsigned mode)
+		       wait_atomic_t_action_f action, unsigned int mode)
 {
 	atomic_t *val;
 	int ret = 0;

@@ -193,7 +193,7 @@ int __wait_on_atomic_t(struct wait_queue_head *wq_head, struct wait_bit_queue_en
 		val = wbq_entry->key.flags;
 		if (atomic_read(val) == 0)
 			break;
-		ret = (*action)(val);
+		ret = (*action)(val, mode);
 	} while (!ret && atomic_read(val) != 0);
 	finish_wait(wq_head, &wbq_entry->wq_entry);
 	return ret;

@@ -210,8 +210,9 @@ int __wait_on_atomic_t(struct wait_queue_head *wq_head, struct wait_bit_queue_en
 		},							\
 	}
 
-__sched int out_of_line_wait_on_atomic_t(atomic_t *p, int (*action)(atomic_t *),
-					 unsigned mode)
+__sched int out_of_line_wait_on_atomic_t(atomic_t *p,
+					 wait_atomic_t_action_f action,
+					 unsigned int mode)
 {
 	struct wait_queue_head *wq_head = atomic_t_waitqueue(p);
 	DEFINE_WAIT_ATOMIC_T(wq_entry, p);

@@ -220,6 +221,15 @@ __sched int out_of_line_wait_on_atomic_t(atomic_t *p, int (*action)(atomic_t *),
 }
 EXPORT_SYMBOL(out_of_line_wait_on_atomic_t);
 
+__sched int atomic_t_wait(atomic_t *counter, unsigned int mode)
+{
+	schedule();
+	if (signal_pending_state(mode, current))
+		return -EINTR;
+	return 0;
+}
+EXPORT_SYMBOL(atomic_t_wait);
+
 /**
  * wake_up_atomic_t - Wake up a waiter on a atomic_t
  * @p: The atomic_t being waited on, a kernel virtual address

David Howells

author maintainer

2017-09-01 15:41 UTC #12 lore raw

RFC PATCH 4/11 Add a function to start/reduce a timer

In reply to David Howells

Add a function, similar to mod_timer(), that will start a timer it isn't running and will modify it if it is running and has an expiry time longer than the new time. If the timer is running with an expiry time that's the same or sooner, no change is made.

The function looks like:

	int reduce_timer(struct timer_list *timer, unsigned long expires);

Signed-off-by David Howells <[email protected]>

cc Thomas Gleixner <[email protected]>

---

 include/linux/timer.h |    1 +
 kernel/time/timer.c   |   49 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 8 deletions(-)

include/linux/timer.h +1 -0

index e6789b8757d5..6ec5d897606d 100644
@@ -187,6 +187,7 @@ extern void add_timer_on(struct timer_list *timer, int cpu);
 extern int del_timer(struct timer_list * timer);
 extern int mod_timer(struct timer_list *timer, unsigned long expires);
 extern int mod_timer_pending(struct timer_list *timer, unsigned long expires);
+extern int reduce_timer(struct timer_list *timer, unsigned long expires);
 
 /*
  * The jiffies value which is added to now, when there is no timer

kernel/time/timer.c +41 -8

102 lines not on this page · raw message

Thomas Gleixner

2017-10-20 12:20 UTC #13 lore raw

Re: [RFC PATCH 04/11] Add a function to start/reduce a timer

Quoted 1 line · David Howells

Add a function, similar to mod_timer(), that will start a timer it isn't

s/it /if it /

Quoted 7 lines

running and will modify it if it is running and has an expiry time longer than the new time. If the timer is running with an expiry time that's the same or sooner, no change is made.
The function looks like:
      int reduce_timer(struct timer_list *timer, unsigned long expires);

Well, yes. But what's the purpose of this function? You explain the what, but not the why.

Quoted 1 line

+extern int reduce_timer(struct timer_list *timer, unsigned long expires);

For new timer functions we really should use the timer_xxxx() convention. The historic naming convention is horrible.

Aside of that timer_reduce() is kinda ugly but I failed to come up with something reasonable as well.

Quoted 9 lines

static inline int

kernel/time/timer.c +1 -1

-__mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
+__mod_timer(struct timer_list *timer, unsigned long expires, unsigned int options)
 {
 	struct timer_base *base, *new_base;
 	unsigned int idx = UINT_MAX;

@@ -938,8 +941,13 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
 	 * same array bucket then just return:
 	 */

kernel/time/timer.c __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only) · line 950

Quoted 10 lines

kernel/time/timer.c +7 -2

 	if (timer_pending(timer)) {
-		if (timer->expires == expires)
-			return 1;
+		if (options & MOD_TIMER_REDUCE) {
+			if (time_before_eq(timer->expires, expires))
+				return 1;
+		} else {
+			if (timer->expires == expires)
+				return 1;
+		}

This hurts the common networking optimzation case. Please keep that check first:

		if (timer->expires == expires)
			return 1;

		if ((options & MOD_TIMER_REDUCE) &&
		    time_before(timer->expires, expires))
		    	return 1;

Also please check whether it's more efficient code wise to have that option thing or if an additional 'bool reduce' argument cerates better code.

Thanks,

tglx

David Howells

author maintainer

2017-11-09 00:33 UTC #14 lore raw

Re: [RFC PATCH 04/11] Add a function to start/reduce a timer

Quoted 7 lines · Thomas Gleixner

Quoted 1 line
+extern int reduce_timer(struct timer_list *timer, unsigned long expires);
For new timer functions we really should use the timer_xxxx() convention. The historic naming convention is horrible.
Aside of that timer_reduce() is kinda ugly but I failed to come up with something reasonable as well.

reduce_timer() sounds snappier, probably because the verb is first, but I can make it timer_reduce() if you prefer. Or maybe timer_advance() - though that's less clear as to the direction.

Quoted 10 lines

kernel/time/timer.c __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only) · line 950
Quoted 7 lines
kernel/time/timer.c +7 -0
+		if (options & MOD_TIMER_REDUCE) {
+			if (time_before_eq(timer->expires, expires))
+				return 1;
+		} else {
+			if (timer->expires == expires)
+				return 1;
+		}
This hurts the common networking optimzation case. Please keep that check first:

The way the code stands, it doesn't make much difference because compiler optimises away the MOD_TIMER_REDUCE case for __mod_timer() and optimises away the other branch for reduce_timer().

Quoted 9 lines

		if (timer->expires == expires)
			return 1;

		if ((options & MOD_TIMER_REDUCE) &&
		    time_before(timer->expires, expires))
		    	return 1;
Also please check whether it's more efficient code wise to have that option thing or if an additional 'bool reduce' argument cerates better code.

It's a constant passed into an inline function - gcc-7's optimiser copes with that for x86_64 at least. mod_timer() contains:

   0xffffffff810bb7a0 <+31>:    cmpq   $0x0,0x8(%rdi)
   0xffffffff810bb7a5 <+36>:    mov    %rsi,%r12
   0xffffffff810bb7a8 <+39>:    mov    %rdi,%rbx
   0xffffffff810bb7ab <+42>:    je     0xffffffff810bb829 <mod_timer+168>
   0xffffffff810bb7ad <+44>:    cmp    0x10(%rdi),%rsi
   0xffffffff810bb7b1 <+48>:    movl   $0x1,-0x38(%rbp)
   0xffffffff810bb7b8 <+55>:    je     0xffffffff810bba9f <mod_timer+798>

and reduce_timer() contains:

   0xffffffff810bbaed <+31>:    cmpq   $0x0,0x8(%rdi)
   0xffffffff810bbaf2 <+36>:    mov    %rsi,%r13
   0xffffffff810bbaf5 <+39>:    mov    %rdi,%rbx
   0xffffffff810bbaf8 <+42>:    je     0xffffffff810bbb9d <reduce_timer+207>
   0xffffffff810bbafe <+48>:    cmp    0x10(%rdi),%rsi
   0xffffffff810bbb02 <+52>:    mov    $0x1,%r14d
   0xffffffff810bbb08 <+58>:    jns    0xffffffff810bbe23 <reduce_timer+853>

As you can see, the relevant jump instruction is JE in one and JNS in the other.

If I make the change you suggest with the equality check being unconditional, mod_timer() is unchanged and reduce_timer() then contains:

   0xffffffff810bbaed <+31>:    cmpq   $0x0,0x8(%rdi)
   0xffffffff810bbaf2 <+36>:    mov    %rsi,%r13
   0xffffffff810bbaf5 <+39>:    mov    %rdi,%rbx
   0xffffffff810bbaf8 <+42>:    je     0xffffffff810bbba9 <reduce_timer+219>
   0xffffffff810bbafe <+48>:    mov    0x10(%rdi),%rax
   0xffffffff810bbb02 <+52>:    mov    $0x1,%r14d
   0xffffffff810bbb08 <+58>:    cmp    %rax,%rsi
   0xffffffff810bbb0b <+61>:    je     0xffffffff810bbe2f <reduce_timer+865>
   0xffffffff810bbb11 <+67>:    cmp    %rax,%rsi
   0xffffffff810bbb14 <+70>:    jns    0xffffffff810bbe2f <reduce_timer+865>

which smacks of a missed optimisation, since timer_before_eq() covers the == case. Doing:

		long diff = timer->expires - expires;
		if (diff == 0)
			return 1;
		if (options & MOD_TIMER_REDUCE &&
		    diff <= 0)
			return 1;

gets me the same code in mod_timer() and the following in reduce_timer():

   0xffffffff810bbaed <+31>:    cmpq   $0x0,0x8(%rdi)
   0xffffffff810bbaf2 <+36>:    mov    %rsi,%r13
   0xffffffff810bbaf5 <+39>:    mov    %rdi,%rbx
   0xffffffff810bbaf8 <+42>:    je     0xffffffff810bbba3 <reduce_timer+213>
   0xffffffff810bbafe <+48>:    mov    0x10(%rdi),%rax
   0xffffffff810bbb02 <+52>:    mov    $0x1,%r14d
   0xffffffff810bbb08 <+58>:    sub    %rsi,%rax
   0xffffffff810bbb0b <+61>:    test   %rax,%rax
   0xffffffff810bbb0e <+64>:    jle    0xffffffff810bbe29 <reduce_timer+859>

which is marginally better - though I think it could still be optimised better by the compiler.

Actually, something that might increase efficiency overall is to make add_timer() an inline and forego the check - but that's a separate matter.

Thanks,
David

David Howells

author maintainer

2017-09-01 15:41 UTC #15 lore raw

RFC PATCH 5/11 afs: Lay the groundwork for supporting network namespaces

In reply to David Howells

Lay the groundwork for supporting network namespaces (netns) to the AFS filesystem by moving various global features to a network-namespace struct (afs_net) and providing an instance of this as a temporary global variable that everything uses via accessor functions for the moment.

The following changes have been made:

(1) Store the netns in the superblock info. This will be obtained from the mounter's nsproxy on a manual mount and inherited from the parent superblock on an automount.

(2) The cell list is made per-netns. It can be viewed through /proc/net/afs/cells and also be modified by writing commands to that file.

(3) The local workstation cell is set per-ns in /proc/net/afs/rootcell. This is unset by default.

(4) The 'rootcell' module parameter, which sets a cell and VL server list modifies the init net namespace, thereby allowing an AFS root fs to be theoretically used.

(5) The volume location lists and the file lock manager are made per-netns.

(6) The AF_RXRPC socket and associated I/O bits are made per-ns.

The various workqueues remain global for the moment.

Changes still to be made:

(1) /proc/fs/afs/ should be moved to /proc/net/afs/ and a symlink emplaced from the old name.

(2) A per-netns subsys needs to be registered for AFS into which it can store its per-netns data.

(3) Rather than the AF_RXRPC socket being opened on module init, it needs to be opened on the creation of a superblock in that netns.

(4) The socket needs to be closed when the last superblock using it is destroyed and all outstanding client calls on it have been completed. This prevents a reference loop on the namespace.

(5) It is possible that several namespaces will want to use AFS, in which case each one will need its own UDP port. These can either be set through /proc/net/afs/cm_port or the kernel can pick one at random. The init_ns gets 7001 by default.

Issues:

(1) The DNS keyring needs net-namespacing.

(2) Where do upcalls go (eg. DNS request-key upcall)?

(3) Need something like open_socket_in_file_ns() syscall so that AFS command line tools attempting to operate on an AFS file/volume have their RPC calls go to the right place.
---

 fs/afs/afs.h               |    9 +++
 fs/afs/callback.c          |   24 +------
 fs/afs/cell.c              |  130 ++++++++++++++++++-------------------
 fs/afs/cmservice.c         |   26 ++++---
 fs/afs/flock.c             |   39 +----------
 fs/afs/fsclient.c          |   56 +++++++++++-----
 fs/afs/internal.h          |  153 ++++++++++++++++++++++++++++++++------------
 fs/afs/main.c              |  153 ++++++++++++++++++++++++++++++--------------
 fs/afs/proc.c              |   64 ++++++++++++------
 fs/afs/rxrpc.c             |  125 ++++++++++++++++++------------------
 fs/afs/server.c            |   82 +++++++++++-------------
 fs/afs/super.c             |   45 +++++++------
 fs/afs/vlclient.c          |   10 ++-
 fs/afs/vlocation.c         |  151 ++++++++++++++++++++-----------------------
 fs/afs/volume.c            |   10 +--
 include/uapi/linux/magic.h |    1 
 16 files changed, 589 insertions(+), 489 deletions(-)

fs/afs/afs.h +9 -0

index 3c462ff6db63..93053115bcfc 100644
@@ -72,6 +72,15 @@ struct afs_callback {
 
 #define AFSCBMAX 50	/* maximum callbacks transferred per bulk op */
 
+struct afs_uuid {
+	__be32		time_low;			/* low part of timestamp */
+	__be16		time_mid;			/* mid part of timestamp */
+	__be16		time_hi_and_version;		/* high part of timestamp and version  */
+	__u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
+	__u8		clock_seq_low;			/* clock seq low */
+	__u8		node[6];			/* spatially unique node ID (MAC addr) */
+};
+
 /*
  * AFS volume information
  */

fs/afs/callback.c +2 ~~-22~~

index 25d404d22cae..d12dffb76b67 100644
@@ -28,9 +28,7 @@ unsigned afs_vnode_update_timeout = 10;
 	CIRC_SPACE((server)->cb_break_head, (server)->cb_break_tail,	\
 		   ARRAY_SIZE((server)->cb_break))
 
-//static void afs_callback_updater(struct work_struct *);
-
-static struct workqueue_struct *afs_callback_update_worker;
+struct workqueue_struct *afs_callback_update_worker;
 
 /*
  * allow the fileserver to request callback state (re-)initialisation

@@ -343,7 +341,7 @@ void afs_dispatch_give_up_callbacks(struct work_struct *work)
 	 *   had callbacks entirely, and the server will call us later to break
 	 *   them
 	 */
-	afs_fs_give_up_callbacks(server, true);
+	afs_fs_give_up_callbacks(server->cell->net, server, true);
 }
 
 /*

@@ -456,21 +454,3 @@ static void afs_callback_updater(struct work_struct *work)
 	afs_put_vnode(vl);
 }
 #endif
-
-/*
- * initialise the callback update process
- */
-int __init afs_callback_update_init(void)
-{
-	afs_callback_update_worker = alloc_ordered_workqueue("kafs_callbackd",
-							     WQ_MEM_RECLAIM);
-	return afs_callback_update_worker ? 0 : -ENOMEM;
-}
-
-/*
- * shut down the callback update process
- */
-void afs_callback_update_kill(void)
-{
-	destroy_workqueue(afs_callback_update_worker);
-}

fs/afs/cell.c +63 ~~-67~~

325 lines not on this page · raw message

fs/afs/cmservice.c +13 ~~-13~~

index 782d4d05a53b..30ce4be4165f 100644
@@ -193,7 +193,7 @@ static int afs_deliver_cb_callback(struct afs_call *call)
 
 	switch (call->unmarshall) {
 	case 0:
-		rxrpc_kernel_get_peer(afs_socket, call->rxcall, &srx);
+		rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
 		call->offset = 0;
 		call->unmarshall++;

@@ -290,7 +290,7 @@ static int afs_deliver_cb_callback(struct afs_call *call)
 
 	/* we'll need the file server record as that tells us which set of
 	 * vnodes to operate upon */
-	server = afs_find_server(&srx);
+	server = afs_find_server(call->net, &srx);
 	if (!server)
 		return -ENOTCONN;
 	call->server = server;

@@ -324,7 +324,7 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call)
 
 	_enter("");
 
-	rxrpc_kernel_get_peer(afs_socket, call->rxcall, &srx);
+	rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
 
 	ret = afs_extract_data(call, NULL, 0, false);
 	if (ret < 0)

@@ -335,7 +335,7 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call)
 
 	/* we'll need the file server record as that tells us which set of
 	 * vnodes to operate upon */
-	server = afs_find_server(&srx);
+	server = afs_find_server(call->net, &srx);
 	if (!server)
 		return -ENOTCONN;
 	call->server = server;

@@ -357,7 +357,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 
 	_enter("");
 
-	rxrpc_kernel_get_peer(afs_socket, call->rxcall, &srx);
+	rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
 
 	_enter("{%u}", call->unmarshall);

@@ -407,7 +407,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 
 	/* we'll need the file server record as that tells us which set of
 	 * vnodes to operate upon */
-	server = afs_find_server(&srx);
+	server = afs_find_server(call->net, &srx);
 	if (!server)
 		return -ENOTCONN;
 	call->server = server;

@@ -461,7 +461,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 
 	_enter("");
 
-	if (memcmp(r, &afs_uuid, sizeof(afs_uuid)) == 0)
+	if (memcmp(r, &call->net->uuid, sizeof(call->net->uuid)) == 0)
 		reply.match = htonl(0);
 	else
 		reply.match = htonl(1);

@@ -568,13 +568,13 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
 	memset(&reply, 0, sizeof(reply));
 	reply.ia.nifs = htonl(nifs);
 
-	reply.ia.uuid[0] = afs_uuid.time_low;
-	reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
-	reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
-	reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
-	reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
+	reply.ia.uuid[0] = call->net->uuid.time_low;
+	reply.ia.uuid[1] = htonl(ntohs(call->net->uuid.time_mid));
+	reply.ia.uuid[2] = htonl(ntohs(call->net->uuid.time_hi_and_version));
+	reply.ia.uuid[3] = htonl((s8) call->net->uuid.clock_seq_hi_and_reserved);
+	reply.ia.uuid[4] = htonl((s8) call->net->uuid.clock_seq_low);
 	for (loop = 0; loop < 6; loop++)
-		reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.node[loop]);
+		reply.ia.uuid[loop + 5] = htonl((s8) call->net->uuid.node[loop]);
 
 	if (ifs) {
 		for (loop = 0; loop < nifs; loop++) {

fs/afs/flock.c +2 ~~-37~~

index 3191dff2c156..559ac00af5f7 100644
@@ -14,48 +14,17 @@
 #define AFS_LOCK_GRANTED	0
 #define AFS_LOCK_PENDING	1
 
+struct workqueue_struct *afs_lock_manager;
+
 static void afs_fl_copy_lock(struct file_lock *new, struct file_lock *fl);
 static void afs_fl_release_private(struct file_lock *fl);
 
-static struct workqueue_struct *afs_lock_manager;
-static DEFINE_MUTEX(afs_lock_manager_mutex);
-
 static const struct file_lock_operations afs_lock_ops = {
 	.fl_copy_lock		= afs_fl_copy_lock,
 	.fl_release_private	= afs_fl_release_private,
 };
 
 /*
- * initialise the lock manager thread if it isn't already running
- */
-static int afs_init_lock_manager(void)
-{
-	int ret;
-
-	ret = 0;
-	if (!afs_lock_manager) {
-		mutex_lock(&afs_lock_manager_mutex);
-		if (!afs_lock_manager) {
-			afs_lock_manager = alloc_workqueue("kafs_lockd",
-							   WQ_MEM_RECLAIM, 0);
-			if (!afs_lock_manager)
-				ret = -ENOMEM;
-		}
-		mutex_unlock(&afs_lock_manager_mutex);
-	}
-	return ret;
-}
-
-/*
- * destroy the lock manager thread if it's running
- */
-void __exit afs_kill_lock_manager(void)
-{
-	if (afs_lock_manager)
-		destroy_workqueue(afs_lock_manager);
-}
-
-/*
  * if the callback is broken on this vnode, then the lock may now be available
  */
 void afs_lock_may_be_available(struct afs_vnode *vnode)

@@ -264,10 +233,6 @@ static int afs_do_setlk(struct file *file, struct file_lock *fl)
 	if (fl->fl_start != 0 || fl->fl_end != OFFSET_MAX)
 		return -EINVAL;
 
-	ret = afs_init_lock_manager();
-	if (ret < 0)
-		return ret;
-
 	fl->fl_ops = &afs_lock_ops;
 	INIT_LIST_HEAD(&fl->fl_u.afs.link);
 	fl->fl_u.afs.state = AFS_LOCK_PENDING;

fs/afs/fsclient.c +37 ~~-19~~

262 lines not on this page · raw message

fs/afs/internal.h +112 ~~-41~~

284 lines not on this page · raw message

fs/afs/main.c +105 ~~-48~~

198 lines not on this page · raw message

fs/afs/proc.c +40 ~~-24~~

170 lines not on this page · raw message

fs/afs/rxrpc.c +61 ~~-64~~

385 lines not on this page · raw message

fs/afs/server.c +39 ~~-43~~

186 lines not on this page · raw message

fs/afs/super.c +26 ~~-19~~

114 lines not on this page · raw message

fs/afs/vlclient.c +6 -4

index a5e4cc561b6c..f5a043a9ba61 100644
@@ -143,7 +143,8 @@ static const struct afs_call_type afs_RXVLGetEntryById = {
 /*
  * dispatch a get volume entry by name operation
  */
-int afs_vl_get_entry_by_name(struct in_addr *addr,
+int afs_vl_get_entry_by_name(struct afs_net *net,
+			     struct in_addr *addr,
 			     struct key *key,
 			     const char *volname,
 			     struct afs_cache_vlocation *entry,

@@ -159,7 +160,7 @@ int afs_vl_get_entry_by_name(struct in_addr *addr,
 	padsz = (4 - (volnamesz & 3)) & 3;
 	reqsz = 8 + volnamesz + padsz;
 
-	call = afs_alloc_flat_call(&afs_RXVLGetEntryByName, reqsz, 384);
+	call = afs_alloc_flat_call(net, &afs_RXVLGetEntryByName, reqsz, 384);
 	if (!call)
 		return -ENOMEM;

@@ -183,7 +184,8 @@ int afs_vl_get_entry_by_name(struct in_addr *addr,
 /*
  * dispatch a get volume entry by ID operation
  */
-int afs_vl_get_entry_by_id(struct in_addr *addr,
+int afs_vl_get_entry_by_id(struct afs_net *net,
+			   struct in_addr *addr,
 			   struct key *key,
 			   afs_volid_t volid,
 			   afs_voltype_t voltype,

@@ -195,7 +197,7 @@ int afs_vl_get_entry_by_id(struct in_addr *addr,
 
 	_enter("");
 
-	call = afs_alloc_flat_call(&afs_RXVLGetEntryById, 12, 384);
+	call = afs_alloc_flat_call(net, &afs_RXVLGetEntryById, 12, 384);
 	if (!call)
 		return -ENOMEM;

fs/afs/vlocation.c +68 ~~-83~~

333 lines not on this page · raw message

fs/afs/volume.c +5 -5

index db73d6dad02b..3d5363e0b7e1 100644
@@ -54,7 +54,7 @@ struct afs_volume *afs_volume_lookup(struct afs_mount_params *params)
 	       params->volnamesz, params->volnamesz, params->volname, params->rwpath);
 
 	/* lookup the volume location record */
-	vlocation = afs_vlocation_lookup(params->cell, params->key,
+	vlocation = afs_vlocation_lookup(params->net, params->cell, params->key,
 					 params->volname, params->volnamesz);
 	if (IS_ERR(vlocation)) {
 		ret = PTR_ERR(vlocation);

@@ -138,7 +138,7 @@ struct afs_volume *afs_volume_lookup(struct afs_mount_params *params)
 	_debug("kAFS selected %s volume %08x",
 	       afs_voltypes[volume->type], volume->vid);
 	up_write(&params->cell->vl_sem);
-	afs_put_vlocation(vlocation);
+	afs_put_vlocation(params->net, vlocation);
 	_leave(" = %p", volume);
 	return volume;

@@ -146,7 +146,7 @@ struct afs_volume *afs_volume_lookup(struct afs_mount_params *params)
 error_up:
 	up_write(&params->cell->vl_sem);
 error:
-	afs_put_vlocation(vlocation);
+	afs_put_vlocation(params->net, vlocation);
 	_leave(" = %d", ret);
 	return ERR_PTR(ret);

@@ -163,7 +163,7 @@ struct afs_volume *afs_volume_lookup(struct afs_mount_params *params)
 /*
  * destroy a volume record
  */
-void afs_put_volume(struct afs_volume *volume)
+void afs_put_volume(struct afs_net *net, struct afs_volume *volume)
 {
 	struct afs_vlocation *vlocation;
 	int loop;

@@ -195,7 +195,7 @@ void afs_put_volume(struct afs_volume *volume)
 #ifdef CONFIG_AFS_FSCACHE
 	fscache_relinquish_cookie(volume->cache, 0);
 #endif
-	afs_put_vlocation(vlocation);
+	afs_put_vlocation(net, vlocation);
 
 	for (loop = volume->nservers - 1; loop >= 0; loop--)
 		afs_put_server(volume->servers[loop]);

include/uapi/linux/magic.h +1 -0

index e439565df838..b61025e1944e 100644
@@ -46,6 +46,7 @@
 #define OPENPROM_SUPER_MAGIC	0x9fa1
 #define QNX4_SUPER_MAGIC	0x002f		/* qnx4 fs detection */
 #define QNX6_SUPER_MAGIC	0x68191122	/* qnx6 fs detection */
+#define AFS_FS_MAGIC		0x6B414653
 
 #define REISERFS_SUPER_MAGIC	0x52654973	/* used by gcc */
 					/* used by file system utilities that

David Howells

author maintainer

2017-09-01 15:41 UTC #16 lore raw

RFC PATCH 6/11 afs: Add some protocol defs

In reply to David Howells

Add some protocol definitions, including max field lengths, flag defs and an XDR-encoded UUID def.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/afs.h    |   25 ++++++++++++++++++++-----
 fs/afs/afs_vl.h |    6 +++++-
 2 files changed, 25 insertions(+), 6 deletions(-)

fs/afs/afs.h +20 -5

index 93053115bcfc..0d837bddbf7d 100644
@@ -14,11 +14,14 @@
 
 #include <linux/in.h>
 
-#define AFS_MAXCELLNAME	64		/* maximum length of a cell name */
-#define AFS_MAXVOLNAME	64		/* maximum length of a volume name */
-#define AFSNAMEMAX	256		/* maximum length of a filename plus NUL */
-#define AFSPATHMAX	1024		/* maximum length of a pathname plus NUL */
-#define AFSOPAQUEMAX	1024		/* maximum length of an opaque field */
+#define AFS_MAXCELLNAME		64  	/* Maximum length of a cell name */
+#define AFS_MAXVOLNAME		64  	/* Maximum length of a volume name */
+#define AFS_MAXNSERVERS		8   	/* Maximum servers in a basic volume record */
+#define AFS_NMAXNSERVERS	13  	/* Maximum servers in a N/U-class volume record */
+#define AFS_MAXTYPES		3	/* Maximum number of volume types */
+#define AFSNAMEMAX		256 	/* Maximum length of a filename plus NUL */
+#define AFSPATHMAX		1024	/* Maximum length of a pathname plus NUL */
+#define AFSOPAQUEMAX		1024	/* Maximum length of an opaque field */
 
 typedef unsigned			afs_volid_t;
 typedef unsigned			afs_vnodeid_t;

@@ -176,4 +179,16 @@ struct afs_volume_status {
 
 #define AFS_BLOCK_SIZE	1024
 
+/*
+ * XDR encoding of UUID in AFS.
+ */
+struct afs_uuid__xdr {
+	__be32		time_low;
+	__be32		time_mid;
+	__be32		time_hi_and_version;
+	__be32		clock_seq_hi_and_reserved;
+	__be32		clock_seq_low;
+	__be32		node[6];
+};
+
 #endif /* AFS_H */

fs/afs/afs_vl.h +5 -1

index 800f607ffaf5..5b7cd9d7e9e6 100644
@@ -74,11 +74,15 @@ struct afs_vldbentry {
 		struct in_addr	addr;		/* server address */
 		unsigned	partition;	/* partition ID on this server */
 		unsigned	flags;		/* server specific flags */
-#define AFS_VLSF_NEWREPSITE	0x0001	/* unused */
+#define AFS_VLSF_NEWREPSITE	0x0001	/* Ignore all 'non-new' servers */
 #define AFS_VLSF_ROVOL		0x0002	/* this server holds a R/O instance of the volume */
 #define AFS_VLSF_RWVOL		0x0004	/* this server holds a R/W instance of the volume */
 #define AFS_VLSF_BACKVOL	0x0008	/* this server holds a backup instance of the volume */
+#define AFS_VLSF_UUID		0x0010	/* This server is referred to by its UUID */
+#define AFS_VLSF_DONTUSE	0x0020	/* This server ref should be ignored */
 	} servers[8];
 };
 
+#define AFS_VLDB_MAXNAMELEN 65
+
 #endif /* AFS_VL_H */

David Howells

author maintainer

2017-09-01 15:41 UTC #17 lore raw

RFC PATCH 7/11 afs: Update the cache index structure

In reply to David Howells

Update the cache index structure in the following ways:

(1) Don't use the volume name followed by the volume type as levels in the cache index. Volumes can be renamed. Use the volume ID instead.

(2) Don't store the VLDB data for a volume in the tree. If the volume database should be cached locally, then it should be done in a separate tree.

(3) Expand the volume ID stored in the cache to 64 bits.

(4) Expand the file/vnode ID stored in the cache to 96 bits.

(5) Increment the cache structure version number to 1.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/cache.c     |  239 +++++++++-------------------------------------------
 fs/afs/internal.h  |   21 -----
 fs/afs/vlocation.c |   39 +-------
 fs/afs/volume.c    |    2 
 4 files changed, 50 insertions(+), 251 deletions(-)

fs/afs/cache.c +43 ~~-196~~

index 577763c3d88b..e19a643e0c29 100644
@@ -14,19 +14,6 @@
 
 static uint16_t afs_cell_cache_get_key(const void *cookie_netfs_data,
 				       void *buffer, uint16_t buflen);
-static uint16_t afs_cell_cache_get_aux(const void *cookie_netfs_data,
-				       void *buffer, uint16_t buflen);
-static enum fscache_checkaux afs_cell_cache_check_aux(void *cookie_netfs_data,
-						      const void *buffer,
-						      uint16_t buflen);
-
-static uint16_t afs_vlocation_cache_get_key(const void *cookie_netfs_data,
-					    void *buffer, uint16_t buflen);
-static uint16_t afs_vlocation_cache_get_aux(const void *cookie_netfs_data,
-					    void *buffer, uint16_t buflen);
-static enum fscache_checkaux afs_vlocation_cache_check_aux(
-	void *cookie_netfs_data, const void *buffer, uint16_t buflen);
-
 static uint16_t afs_volume_cache_get_key(const void *cookie_netfs_data,
 					 void *buffer, uint16_t buflen);

@@ -43,23 +30,13 @@ static void afs_vnode_cache_now_uncached(void *cookie_netfs_data);
 
 struct fscache_netfs afs_cache_netfs = {
 	.name			= "afs",
-	.version		= 0,
+	.version		= 1,
 };
 
 struct fscache_cookie_def afs_cell_cache_index_def = {
 	.name		= "AFS.cell",
 	.type		= FSCACHE_COOKIE_TYPE_INDEX,
 	.get_key	= afs_cell_cache_get_key,
-	.get_aux	= afs_cell_cache_get_aux,
-	.check_aux	= afs_cell_cache_check_aux,
-};
-
-struct fscache_cookie_def afs_vlocation_cache_index_def = {
-	.name			= "AFS.vldb",
-	.type			= FSCACHE_COOKIE_TYPE_INDEX,
-	.get_key		= afs_vlocation_cache_get_key,
-	.get_aux		= afs_vlocation_cache_get_aux,
-	.check_aux		= afs_vlocation_cache_check_aux,
 };
 
 struct fscache_cookie_def afs_volume_cache_index_def = {

@@ -97,150 +74,26 @@ static uint16_t afs_cell_cache_get_key(const void *cookie_netfs_data,
 	return klen;
 }
 
-/*
- * provide new auxiliary cache data
- */
-static uint16_t afs_cell_cache_get_aux(const void *cookie_netfs_data,
-				       void *buffer, uint16_t bufmax)
-{
-	const struct afs_cell *cell = cookie_netfs_data;
-	uint16_t dlen;
-
-	_enter("%p,%p,%u", cell, buffer, bufmax);
-
-	dlen = cell->vl_naddrs * sizeof(cell->vl_addrs[0]);
-	dlen = min(dlen, bufmax);
-	dlen &= ~(sizeof(cell->vl_addrs[0]) - 1);
-
-	memcpy(buffer, cell->vl_addrs, dlen);
-	return dlen;
-}
-
-/*
- * check that the auxiliary data indicates that the entry is still valid
- */
-static enum fscache_checkaux afs_cell_cache_check_aux(void *cookie_netfs_data,
-						      const void *buffer,
-						      uint16_t buflen)
-{
-	_leave(" = OKAY");
-	return FSCACHE_CHECKAUX_OKAY;
-}
-
-/*****************************************************************************/
-/*
- * set the key for the index entry
- */
-static uint16_t afs_vlocation_cache_get_key(const void *cookie_netfs_data,
-					    void *buffer, uint16_t bufmax)
-{
-	const struct afs_vlocation *vlocation = cookie_netfs_data;
-	uint16_t klen;
-
-	_enter("{%s},%p,%u", vlocation->vldb.name, buffer, bufmax);
-
-	klen = strnlen(vlocation->vldb.name, sizeof(vlocation->vldb.name));
-	if (klen > bufmax)
-		return 0;
-
-	memcpy(buffer, vlocation->vldb.name, klen);
-
-	_leave(" = %u", klen);
-	return klen;
-}
-
-/*
- * provide new auxiliary cache data
- */
-static uint16_t afs_vlocation_cache_get_aux(const void *cookie_netfs_data,
-					    void *buffer, uint16_t bufmax)
-{
-	const struct afs_vlocation *vlocation = cookie_netfs_data;
-	uint16_t dlen;
-
-	_enter("{%s},%p,%u", vlocation->vldb.name, buffer, bufmax);
-
-	dlen = sizeof(struct afs_cache_vlocation);
-	dlen -= offsetof(struct afs_cache_vlocation, nservers);
-	if (dlen > bufmax)
-		return 0;
-
-	memcpy(buffer, (uint8_t *)&vlocation->vldb.nservers, dlen);
-
-	_leave(" = %u", dlen);
-	return dlen;
-}
-
-/*
- * check that the auxiliary data indicates that the entry is still valid
- */
-static
-enum fscache_checkaux afs_vlocation_cache_check_aux(void *cookie_netfs_data,
-						    const void *buffer,
-						    uint16_t buflen)
-{
-	const struct afs_cache_vlocation *cvldb;
-	struct afs_vlocation *vlocation = cookie_netfs_data;
-	uint16_t dlen;
-
-	_enter("{%s},%p,%u", vlocation->vldb.name, buffer, buflen);
-
-	/* check the size of the data is what we're expecting */
-	dlen = sizeof(struct afs_cache_vlocation);
-	dlen -= offsetof(struct afs_cache_vlocation, nservers);
-	if (dlen != buflen)
-		return FSCACHE_CHECKAUX_OBSOLETE;
-
-	cvldb = container_of(buffer, struct afs_cache_vlocation, nservers);
-
-	/* if what's on disk is more valid than what's in memory, then use the
-	 * VL record from the cache */
-	if (!vlocation->valid || vlocation->vldb.rtime == cvldb->rtime) {
-		memcpy((uint8_t *)&vlocation->vldb.nservers, buffer, dlen);
-		vlocation->valid = 1;
-		_leave(" = SUCCESS [c->m]");
-		return FSCACHE_CHECKAUX_OKAY;
-	}
-
-	/* need to update the cache if the cached info differs */
-	if (memcmp(&vlocation->vldb, buffer, dlen) != 0) {
-		/* delete if the volume IDs for this name differ */
-		if (memcmp(&vlocation->vldb.vid, &cvldb->vid,
-			   sizeof(cvldb->vid)) != 0
-		    ) {
-			_leave(" = OBSOLETE");
-			return FSCACHE_CHECKAUX_OBSOLETE;
-		}
-
-		_leave(" = UPDATE");
-		return FSCACHE_CHECKAUX_NEEDS_UPDATE;
-	}
-
-	_leave(" = OKAY");
-	return FSCACHE_CHECKAUX_OKAY;
-}
-
 /*****************************************************************************/
 /*
  * set the key for the volume index entry
  */
 static uint16_t afs_volume_cache_get_key(const void *cookie_netfs_data,
-					void *buffer, uint16_t bufmax)
+					 void *buffer, uint16_t bufmax)
 {
 	const struct afs_volume *volume = cookie_netfs_data;
-	uint16_t klen;
+	struct {
+		u64 volid;
+	} __packed key;
 
 	_enter("{%u},%p,%u", volume->type, buffer, bufmax);
 
-	klen = sizeof(volume->type);
-	if (klen > bufmax)
+	if (bufmax < sizeof(key))
 		return 0;
 
-	memcpy(buffer, &volume->type, sizeof(volume->type));
-
-	_leave(" = %u", klen);
-	return klen;
-
+	key.volid = volume->vid;
+	memcpy(buffer, &key, sizeof(key));
+	return sizeof(key);
 }
 
 /*****************************************************************************/

@@ -251,20 +104,25 @@ static uint16_t afs_vnode_cache_get_key(const void *cookie_netfs_data,
 					void *buffer, uint16_t bufmax)
 {
 	const struct afs_vnode *vnode = cookie_netfs_data;
-	uint16_t klen;
+	struct {
+		u32 vnode_id[3];
+	} __packed key;
 
 	_enter("{%x,%x,%llx},%p,%u",
 	       vnode->fid.vnode, vnode->fid.unique, vnode->status.data_version,
 	       buffer, bufmax);
 
-	klen = sizeof(vnode->fid.vnode);
-	if (klen > bufmax)
-		return 0;
+	/* Allow for a 96-bit key */
+	memset(&key, 0, sizeof(key));
+	key.vnode_id[0] = vnode->fid.vnode;
+	key.vnode_id[1] = 0;
+	key.vnode_id[2] = 0;
 
-	memcpy(buffer, &vnode->fid.vnode, sizeof(vnode->fid.vnode));
+	if (sizeof(key) > bufmax)
+		return 0;
 
-	_leave(" = %u", klen);
-	return klen;
+	memcpy(buffer, &key, sizeof(key));
+	return sizeof(key);
 }
 
 /*

@@ -282,6 +140,11 @@ static void afs_vnode_cache_get_attr(const void *cookie_netfs_data,
 	*size = vnode->status.size;
 }
 
+struct afs_vnode_cache_aux {
+	u64 data_version;
+	u32 fid_unique;
+} __packed;
+
 /*
  * provide new auxiliary cache data
  */

@@ -289,23 +152,21 @@ static uint16_t afs_vnode_cache_get_aux(const void *cookie_netfs_data,
 					void *buffer, uint16_t bufmax)
 {
 	const struct afs_vnode *vnode = cookie_netfs_data;
-	uint16_t dlen;
+	struct afs_vnode_cache_aux aux;
 
 	_enter("{%x,%x,%Lx},%p,%u",
 	       vnode->fid.vnode, vnode->fid.unique, vnode->status.data_version,
 	       buffer, bufmax);
 
-	dlen = sizeof(vnode->fid.unique) + sizeof(vnode->status.data_version);
-	if (dlen > bufmax)
-		return 0;
+	memset(&aux, 0, sizeof(aux));
+	aux.data_version = vnode->status.data_version;
+	aux.fid_unique = vnode->fid.unique;
 
-	memcpy(buffer, &vnode->fid.unique, sizeof(vnode->fid.unique));
-	buffer += sizeof(vnode->fid.unique);
-	memcpy(buffer, &vnode->status.data_version,
-	       sizeof(vnode->status.data_version));
+	if (bufmax < sizeof(aux))
+		return 0;
 
-	_leave(" = %u", dlen);
-	return dlen;
+	memcpy(buffer, &aux, sizeof(aux));
+	return sizeof(aux);
 }
 
 /*

@@ -316,43 +177,29 @@ static enum fscache_checkaux afs_vnode_cache_check_aux(void *cookie_netfs_data,
 						       uint16_t buflen)
 {
 	struct afs_vnode *vnode = cookie_netfs_data;
-	uint16_t dlen;
+	struct afs_vnode_cache_aux aux;
 
 	_enter("{%x,%x,%llx},%p,%u",
 	       vnode->fid.vnode, vnode->fid.unique, vnode->status.data_version,
 	       buffer, buflen);
 
+	memcpy(&aux, buffer, sizeof(aux));
+
 	/* check the size of the data is what we're expecting */
-	dlen = sizeof(vnode->fid.unique) + sizeof(vnode->status.data_version);
-	if (dlen != buflen) {
-		_leave(" = OBSOLETE [len %hx != %hx]", dlen, buflen);
+	if (buflen != sizeof(aux)) {
+		_leave(" = OBSOLETE [len %hx != %zx]", buflen, sizeof(aux));
 		return FSCACHE_CHECKAUX_OBSOLETE;
 	}
 
-	if (memcmp(buffer,
-		   &vnode->fid.unique,
-		   sizeof(vnode->fid.unique)
-		   ) != 0) {
-		unsigned unique;
-
-		memcpy(&unique, buffer, sizeof(unique));
-
+	if (vnode->fid.unique != aux.fid_unique) {
 		_leave(" = OBSOLETE [uniq %x != %x]",
-		       unique, vnode->fid.unique);
+		       aux.fid_unique, vnode->fid.unique);
 		return FSCACHE_CHECKAUX_OBSOLETE;
 	}
 
-	if (memcmp(buffer + sizeof(vnode->fid.unique),
-		   &vnode->status.data_version,
-		   sizeof(vnode->status.data_version)
-		   ) != 0) {
-		afs_dataversion_t version;
-
-		memcpy(&version, buffer + sizeof(vnode->fid.unique),
-		       sizeof(version));
-
+	if (vnode->status.data_version != aux.data_version) {
 		_leave(" = OBSOLETE [vers %llx != %llx]",
-		       version, vnode->status.data_version);
+		       aux.data_version, vnode->status.data_version);
 		return FSCACHE_CHECKAUX_OBSOLETE;
 	}

fs/afs/internal.h +0 ~~-21~~

index e0484a38c9ce..0ffdb02a58cb 100644
@@ -189,14 +189,6 @@ static inline struct afs_super_info *AFS_FS_S(struct super_block *sb)
 extern struct file_system_type afs_fs_type;
 
 /*
- * entry in the cached cell catalogue
- */
-struct afs_cache_cell {
-	char		name[AFS_MAXCELLNAME];	/* cell name (padded with NULs) */
-	struct in_addr	vl_servers[15];		/* cached cell VL servers */
-};
-
-/*
  * AFS network namespace record.
  */
 struct afs_net {

@@ -293,14 +285,6 @@ struct afs_cache_vlocation {
 };
 
 /*
- * volume -> vnode hash table entry
- */
-struct afs_cache_vhash {
-	afs_voltype_t		vtype;		/* which volume variation */
-	uint8_t			hash_bucket;	/* which hash bucket this represents */
-} __attribute__((packed));
-
-/*
  * AFS volume location record
  */
 struct afs_vlocation {

@@ -310,9 +294,6 @@ struct afs_vlocation {
 	struct list_head	grave;		/* link in master graveyard list */
 	struct list_head	update;		/* link in master update list */
 	struct afs_cell		*cell;		/* cell to which volume belongs */
-#ifdef CONFIG_AFS_FSCACHE
-	struct fscache_cookie	*cache;		/* caching cookie */
-#endif
 	struct afs_cache_vlocation vldb;	/* volume information DB record */
 	struct afs_volume	*vols[3];	/* volume access record pointer (index by type) */
 	wait_queue_head_t	waitq;		/* status change waitqueue */

@@ -472,12 +453,10 @@ struct afs_interface {
 #ifdef CONFIG_AFS_FSCACHE
 extern struct fscache_netfs afs_cache_netfs;
 extern struct fscache_cookie_def afs_cell_cache_index_def;
-extern struct fscache_cookie_def afs_vlocation_cache_index_def;
 extern struct fscache_cookie_def afs_volume_cache_index_def;
 extern struct fscache_cookie_def afs_vnode_cache_index_def;
 #else
 #define afs_cell_cache_index_def	(*(struct fscache_cookie_def *) NULL)
-#define afs_vlocation_cache_index_def	(*(struct fscache_cookie_def *) NULL)
 #define afs_volume_cache_index_def	(*(struct fscache_cookie_def *) NULL)
 #define afs_vnode_cache_index_def	(*(struct fscache_cookie_def *) NULL)
 #endif

fs/afs/vlocation.c +6 ~~-33~~

index ccb7aacfbeca..d9a5d7acdb86 100644
@@ -273,10 +273,6 @@ static void afs_vlocation_apply_update(struct afs_vlocation *vl,
 		       vl->vldb.name, vldb->name);
 
 	vl->vldb = *vldb;
-
-#ifdef CONFIG_AFS_FSCACHE
-	fscache_update_cookie(vl->cache);
-#endif
 }
 
 /*

@@ -295,27 +291,12 @@ static int afs_vlocation_fill_in_record(struct afs_vlocation *vl,
 
 	memset(&vldb, 0, sizeof(vldb));
 
-	/* see if we have an in-cache copy (will set vl->valid if there is) */
-#ifdef CONFIG_AFS_FSCACHE
-	vl->cache = fscache_acquire_cookie(vl->cell->cache,
-					   &afs_vlocation_cache_index_def, vl,
-					   true);
-#endif
-
-	if (vl->valid) {
-		/* try to update a known volume in the cell VL databases by
-		 * ID as the name may have changed */
-		_debug("found in cache");
-		ret = afs_vlocation_update_record(vl, key, &vldb);
-	} else {
-		/* try to look up an unknown volume in the cell VL databases by
-		 * name */
-		ret = afs_vlocation_access_vl_by_name(vl, key, &vldb);
-		if (ret < 0) {
-			printk("kAFS: failed to locate '%s' in cell '%s'\n",
-			       vl->vldb.name, vl->cell->name);
-			return ret;
-		}
+	/* Try to look up an unknown volume in the cell VL databases by name */
+	ret = afs_vlocation_access_vl_by_name(vl, key, &vldb);
+	if (ret < 0) {
+		printk("kAFS: failed to locate '%s' in cell '%s'\n",
+		       vl->vldb.name, vl->cell->name);
+		return ret;
 	}
 
 	afs_vlocation_apply_update(vl, &vldb);

@@ -414,11 +395,6 @@ struct afs_vlocation *afs_vlocation_lookup(struct afs_net *net,
 	spin_unlock(&vl->lock);
 	wake_up(&vl->waitq);
 
-	/* update volume entry in local cache */
-#ifdef CONFIG_AFS_FSCACHE
-	fscache_update_cookie(vl->cache);
-#endif
-
 	/* schedule for regular updates */
 	afs_vlocation_queue_for_updates(net, vl);
 	goto success;

@@ -522,9 +498,6 @@ static void afs_vlocation_destroy(struct afs_vlocation *vl)
 {
 	_enter("%p", vl);
 
-#ifdef CONFIG_AFS_FSCACHE
-	fscache_relinquish_cookie(vl->cache, 0);
-#endif
 	afs_put_cell(vl->cell);
 	kfree(vl);
 }

fs/afs/volume.c +1 -1

index 3d5363e0b7e1..bc52203695b7 100644
@@ -125,7 +125,7 @@ struct afs_volume *afs_volume_lookup(struct afs_mount_params *params)
 
 	/* attach the cache and volume location */
 #ifdef CONFIG_AFS_FSCACHE
-	volume->cache = fscache_acquire_cookie(vlocation->cache,
+	volume->cache = fscache_acquire_cookie(volume->cell->cache,
 					       &afs_volume_cache_index_def,
 					       volume, true);
 #endif

David Howells

author maintainer

2017-09-01 15:41 UTC #18 lore raw

RFC PATCH 8/11 afs: Keep and pass sockaddr_rxrpc addresses rather than in_addr

In reply to David Howells

Keep and pass sockaddr_rxrpc addresses around rather than keeping and passing in_addr addresses to allow for the use of IPv6 and non-standard port numbers in future.

This also allows the port and service_id fields to be removed from the afs_call struct.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/cell.c      |   18 +++++++++++++++---
 fs/afs/fsclient.c  |   36 ------------------------------------
 fs/afs/internal.h  |   16 +++++++---------
 fs/afs/proc.c      |   10 +++++-----
 fs/afs/rxrpc.c     |   18 ++++--------------
 fs/afs/server.c    |   31 ++++++++++++++++---------------
 fs/afs/vlclient.c  |   20 ++++++++++++--------
 fs/afs/vlocation.c |   30 ++++++++----------------------
 fs/afs/vnode.c     |   28 ++++++++++++++--------------
 fs/afs/volume.c    |    9 ++++-----
 10 files changed, 85 insertions(+), 131 deletions(-)

fs/afs/cell.c +15 -3

index bd570fa539a0..3a6c91ae11a3 100644
@@ -31,7 +31,7 @@ static struct afs_cell *afs_cell_alloc(struct afs_net *net,
 	char keyname[4 + AFS_MAXCELLNAME + 1], *cp, *dp, *next;
 	char  *dvllist = NULL, *_vllist = NULL;
 	char  delimiter = ':';
-	int ret;
+	int ret, i;
 
 	_enter("%*.*s,%s", namelen, namelen, name ?: "", vllist);

@@ -61,6 +61,14 @@ static struct afs_cell *afs_cell_alloc(struct afs_net *net,
 	INIT_LIST_HEAD(&cell->vl_list);
 	spin_lock_init(&cell->vl_lock);
 
+	for (i = 0; i < AFS_CELL_MAX_ADDRS; i++) {
+		struct sockaddr_rxrpc *srx = &cell->vl_addrs[i];
+		srx->srx_family			= AF_RXRPC;
+		srx->srx_service		= VL_SERVICE;
+		srx->transport_type		= SOCK_DGRAM;
+		srx->transport.sin.sin_port	= htons(AFS_VL_PORT);
+	}
+
 	/* if the ip address is invalid, try dns query */
 	if (!vllist || strlen(vllist) < 7) {
 		ret = dns_query("afsdb", name, namelen, "ipv4", &dvllist, NULL);

@@ -83,6 +91,7 @@ static struct afs_cell *afs_cell_alloc(struct afs_net *net,
 
 	/* fill in the VL server list from the rest of the string */
 	do {
+		struct sockaddr_rxrpc *srx = &cell->vl_addrs[cell->vl_naddrs];
 		unsigned a, b, c, d;
 
 		next = strchr(_vllist, delimiter);

@@ -95,10 +104,13 @@ static struct afs_cell *afs_cell_alloc(struct afs_net *net,
 		if (a > 255 || b > 255 || c > 255 || d > 255)
 			goto bad_address;
 
-		cell->vl_addrs[cell->vl_naddrs++].s_addr =
+		srx->transport_len		= sizeof(struct sockaddr_in);
+		srx->transport.sin.sin_family	= AF_INET;
+		srx->transport.sin.sin_addr.s_addr =
 			htonl((a << 24) | (b << 16) | (c << 8) | d);
 
-	} while (cell->vl_naddrs < AFS_CELL_MAX_ADDRS && (_vllist = next));
+	} while (cell->vl_naddrs++,
+		 cell->vl_naddrs < AFS_CELL_MAX_ADDRS && (_vllist = next));
 
 	/* create a key to represent an anonymous user */
 	memcpy(keyname, "afs@", 4);

fs/afs/fsclient.c +0 ~~-36~~

index ce6f0159e1d4..bac2e8db6e75 100644
@@ -297,8 +297,6 @@ int afs_fs_fetch_file_status(struct afs_server *server,
 	call->key = key;
 	call->reply = vnode;
 	call->reply2 = volsync;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -504,8 +502,6 @@ static int afs_fs_fetch_data64(struct afs_server *server,
 	call->reply = vnode;
 	call->reply2 = NULL; /* volsync */
 	call->reply3 = req;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->operation_ID = FSFETCHDATA64;
 
 	/* marshall the parameters */

@@ -551,8 +547,6 @@ int afs_fs_fetch_data(struct afs_server *server,
 	call->reply = vnode;
 	call->reply2 = NULL; /* volsync */
 	call->reply3 = req;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->operation_ID = FSFETCHDATA;
 
 	/* marshall the parameters */

@@ -619,8 +613,6 @@ int afs_fs_give_up_callbacks(struct afs_net *net,
 	if (!call)
 		return -ENOMEM;
 
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -723,8 +715,6 @@ int afs_fs_create(struct afs_server *server,
 	call->reply2 = newfid;
 	call->reply3 = newstatus;
 	call->reply4 = newcb;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -810,8 +800,6 @@ int afs_fs_remove(struct afs_server *server,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -893,8 +881,6 @@ int afs_fs_link(struct afs_server *server,
 	call->key = key;
 	call->reply = dvnode;
 	call->reply2 = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -988,8 +974,6 @@ int afs_fs_symlink(struct afs_server *server,
 	call->reply = vnode;
 	call->reply2 = newfid;
 	call->reply3 = newstatus;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -1094,8 +1078,6 @@ int afs_fs_rename(struct afs_server *server,
 	call->key = key;
 	call->reply = orig_dvnode;
 	call->reply2 = new_dvnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -1196,8 +1178,6 @@ static int afs_fs_store_data64(struct afs_server *server,
 	call->wb = wb;
 	call->key = wb->key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->mapping = vnode->vfs_inode.i_mapping;
 	call->first = first;
 	call->last = last;

@@ -1274,8 +1254,6 @@ int afs_fs_store_data(struct afs_server *server, struct afs_writeback *wb,
 	call->wb = wb;
 	call->key = wb->key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->mapping = vnode->vfs_inode.i_mapping;
 	call->first = first;
 	call->last = last;

@@ -1383,8 +1361,6 @@ static int afs_fs_setattr_size64(struct afs_server *server, struct key *key,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->store_version = vnode->status.data_version + 1;
 	call->operation_ID = FSSTOREDATA;

@@ -1435,8 +1411,6 @@ static int afs_fs_setattr_size(struct afs_server *server, struct key *key,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->store_version = vnode->status.data_version + 1;
 	call->operation_ID = FSSTOREDATA;

@@ -1483,8 +1457,6 @@ int afs_fs_setattr(struct afs_server *server, struct key *key,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 	call->operation_ID = FSSTORESTATUS;
 
 	/* marshall the parameters */

@@ -1721,8 +1693,6 @@ int afs_fs_get_volume_status(struct afs_server *server,
 	call->reply = vnode;
 	call->reply2 = vs;
 	call->reply3 = tmpbuf;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -1805,8 +1775,6 @@ int afs_fs_set_lock(struct afs_server *server,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -1839,8 +1807,6 @@ int afs_fs_extend_lock(struct afs_server *server,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -1872,8 +1838,6 @@ int afs_fs_release_lock(struct afs_server *server,
 
 	call->key = key;
 	call->reply = vnode;
-	call->service_id = FS_SERVICE;
-	call->port = htons(AFS_FS_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

fs/afs/internal.h +7 -9

index 0ffdb02a58cb..47d5ae08f071 100644
@@ -104,8 +104,6 @@ struct afs_call {
 	bool			send_pages;	/* T if data from mapping should be sent */
 	bool			need_attention;	/* T if RxRPC poked us */
 	bool			async;		/* T if asynchronous */
-	u16			service_id;	/* RxRPC service ID to call */
-	__be16			port;		/* target UDP port */
 	u32			operation_ID;	/* operation ID for an incoming call */
 	u32			count;		/* count for use in unmarshalling */
 	__be32			tmp;		/* place to extract temporary data */

@@ -260,7 +258,7 @@ struct afs_cell {
 	spinlock_t		vl_lock;	/* vl_list lock */
 	unsigned short		vl_naddrs;	/* number of VL servers in addr list */
 	unsigned short		vl_curr_svix;	/* current server index */
-	struct in_addr		vl_addrs[AFS_CELL_MAX_ADDRS];	/* cell VL server addresses */
+	struct sockaddr_rxrpc	vl_addrs[AFS_CELL_MAX_ADDRS];	/* cell VL server addresses */
 
 	char			name[0];	/* cell name - must go last */
 };

@@ -280,7 +278,7 @@ struct afs_cache_vlocation {
 #define AFS_VOL_VTM_BAK	0x04 /* backup version of the volume is available (on this server) */
 
 	afs_volid_t		vid[3];		/* volume IDs for R/W, R/O and Bak volumes */
-	struct in_addr		servers[8];	/* fileserver addresses */
+	struct sockaddr_rxrpc	servers[8];	/* fileserver addresses */
 	time_t			rtime;		/* last retrieval time */
 };

@@ -311,7 +309,7 @@ struct afs_vlocation {
 struct afs_server {
 	atomic_t		usage;
 	time64_t		time_of_death;	/* time at which put reduced usage to 0 */
-	struct in_addr		addr;		/* server address */
+	struct sockaddr_rxrpc	addr;		/* server address */
 	struct afs_cell		*cell;		/* cell in which server resides */
 	struct list_head	link;		/* link in cell's server list */
 	struct list_head	grave;		/* link in master graveyard list */

@@ -634,7 +632,7 @@ extern void __net_exit afs_close_socket(struct afs_net *);
 extern void afs_charge_preallocation(struct work_struct *);
 extern void afs_put_call(struct afs_call *);
 extern int afs_queue_call_work(struct afs_call *);
-extern int afs_make_call(struct in_addr *, struct afs_call *, gfp_t, bool);
+extern int afs_make_call(struct sockaddr_rxrpc *, struct afs_call *, gfp_t, bool);
 extern struct afs_call *afs_alloc_flat_call(struct afs_net *,
 					    const struct afs_call_type *,
 					    size_t, size_t);

@@ -669,7 +667,7 @@ do {								\
 } while(0)
 
 extern struct afs_server *afs_lookup_server(struct afs_cell *,
-					    const struct in_addr *);
+					    struct sockaddr_rxrpc *);
 extern struct afs_server *afs_find_server(struct afs_net *,
 					  const struct sockaddr_rxrpc *);
 extern void afs_put_server(struct afs_server *);

@@ -686,11 +684,11 @@ extern void __exit afs_fs_exit(void);
  * vlclient.c
  */
 extern int afs_vl_get_entry_by_name(struct afs_net *,
-				    struct in_addr *, struct key *,
+				    struct sockaddr_rxrpc *, struct key *,
 				    const char *, struct afs_cache_vlocation *,
 				    bool);
 extern int afs_vl_get_entry_by_id(struct afs_net *,
-				  struct in_addr *, struct key *,
+				  struct sockaddr_rxrpc *, struct key *,
 				  afs_volid_t, afs_voltype_t,
 				  struct afs_cache_vlocation *, bool);

fs/afs/proc.c +5 -5

index c93433460348..4d609869a57b 100644
@@ -570,16 +570,16 @@ static void afs_proc_cell_vlservers_stop(struct seq_file *p, void *v)
  */
 static int afs_proc_cell_vlservers_show(struct seq_file *m, void *v)
 {
-	struct in_addr *addr = v;
+	struct sockaddr_rxrpc *addr = v;
 
 	/* display header on line 1 */
-	if (v == (struct in_addr *) 1) {
+	if (v == (void *)1) {
 		seq_puts(m, "ADDRESS\n");
 		return 0;
 	}
 
 	/* display one cell per line on subsequent lines */
-	seq_printf(m, "%pI4\n", &addr->s_addr);
+	seq_printf(m, "%pISp\n", &addr->transport);
 	return 0;
 }

@@ -652,7 +652,7 @@ static int afs_proc_cell_servers_show(struct seq_file *m, void *v)
 {
 	struct afs_cell *cell = m->private;
 	struct afs_server *server = list_entry(v, struct afs_server, link);
-	char ipaddr[20];
+	char ipaddr[64];
 
 	/* display header on line 1 */
 	if (v == &cell->servers) {

@@ -661,7 +661,7 @@ static int afs_proc_cell_servers_show(struct seq_file *m, void *v)
 	}
 
 	/* display one cell per line on subsequent lines */
-	sprintf(ipaddr, "%pI4", &server->addr);
+	sprintf(ipaddr, "%pISp", &server->addr.transport);
 	seq_printf(m, "%3d %-15.15s %5d\n",
 		   atomic_read(&server->usage), ipaddr, server->fs_state);

fs/afs/rxrpc.c +4 ~~-14~~

index fba7d56b64b4..f57a2d41a310 100644
@@ -322,10 +322,9 @@ static int afs_send_pages(struct afs_call *call, struct msghdr *msg)
 /*
  * initiate a call
  */
-int afs_make_call(struct in_addr *addr, struct afs_call *call, gfp_t gfp,
-		  bool async)
+int afs_make_call(struct sockaddr_rxrpc *srx, struct afs_call *call,
+		  gfp_t gfp, bool async)
 {
-	struct sockaddr_rxrpc srx;
 	struct rxrpc_call *rxcall;
 	struct msghdr msg;
 	struct kvec iov[1];

@@ -334,7 +333,7 @@ int afs_make_call(struct in_addr *addr, struct afs_call *call, gfp_t gfp,
 	u32 abort_code;
 	int ret;
 
-	_enter("%x,{%d},", addr->s_addr, ntohs(call->port));
+	_enter(",{%pISp},", &srx->transport);
 
 	ASSERT(call->type != NULL);
 	ASSERT(call->type->name != NULL);

@@ -345,15 +344,6 @@ int afs_make_call(struct in_addr *addr, struct afs_call *call, gfp_t gfp,
 
 	call->async = async;
 
-	memset(&srx, 0, sizeof(srx));
-	srx.srx_family = AF_RXRPC;
-	srx.srx_service = call->service_id;
-	srx.transport_type = SOCK_DGRAM;
-	srx.transport_len = sizeof(srx.transport.sin);
-	srx.transport.sin.sin_family = AF_INET;
-	srx.transport.sin.sin_port = call->port;
-	memcpy(&srx.transport.sin.sin_addr, addr, 4);
-
 	/* Work out the length we're going to transmit.  This is awkward for
 	 * calls such as FS.StoreData where there's an extra injection of data
 	 * after the initial fixed part.

@@ -365,7 +355,7 @@ int afs_make_call(struct in_addr *addr, struct afs_call *call, gfp_t gfp,
 	}
 
 	/* create a call */
-	rxcall = rxrpc_kernel_begin_call(call->net->socket, &srx, call->key,
+	rxcall = rxrpc_kernel_begin_call(call->net->socket, srx, call->key,
 					 (unsigned long)call,
 					 tx_total_len, gfp,
 					 (async ?

fs/afs/server.c +16 ~~-15~~

index e47fd9bc0ddc..7d103321efab 100644
@@ -11,6 +11,7 @@
 
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include "afs_fs.h"
 #include "internal.h"
 
 static unsigned afs_server_timeout = 10;	/* server timeout in seconds */

@@ -23,7 +24,7 @@ static int afs_install_server(struct afs_server *server)
 	struct afs_server *xserver;
 	struct afs_net *net = server->cell->net;
 	struct rb_node **pp, *p;
-	int ret;
+	int ret, diff;
 
 	_enter("%p", server);

@@ -36,9 +37,10 @@ static int afs_install_server(struct afs_server *server)
 		p = *pp;
 		_debug("- consider %p", p);
 		xserver = rb_entry(p, struct afs_server, master_rb);
-		if (server->addr.s_addr < xserver->addr.s_addr)
+		diff = memcmp(&server->addr, &xserver->addr, sizeof(server->addr));
+		if (diff < 0)
 			pp = &(*pp)->rb_left;
-		else if (server->addr.s_addr > xserver->addr.s_addr)
+		else if (diff > 0)
 			pp = &(*pp)->rb_right;
 		else
 			goto error;

@@ -57,7 +59,7 @@ static int afs_install_server(struct afs_server *server)
  * allocate a new server record
  */
 static struct afs_server *afs_alloc_server(struct afs_cell *cell,
-					   const struct in_addr *addr)
+					   const struct sockaddr_rxrpc *addr)
 {
 	struct afs_server *server;

@@ -79,8 +81,7 @@ static struct afs_server *afs_alloc_server(struct afs_cell *cell,
 		INIT_DELAYED_WORK(&server->cb_break_work,
 				  afs_dispatch_give_up_callbacks);
 
-		memcpy(&server->addr, addr, sizeof(struct in_addr));
-		server->addr.s_addr = addr->s_addr;
+		server->addr = *addr;
 		_leave(" = %p{%d}", server, atomic_read(&server->usage));
 	} else {
 		_leave(" = NULL [nomem]");

@@ -92,17 +93,17 @@ static struct afs_server *afs_alloc_server(struct afs_cell *cell,
  * get an FS-server record for a cell
  */
 struct afs_server *afs_lookup_server(struct afs_cell *cell,
-				     const struct in_addr *addr)
+				     struct sockaddr_rxrpc *addr)
 {
 	struct afs_server *server, *candidate;
 
-	_enter("%p,%pI4", cell, &addr->s_addr);
+	_enter("%p,%pIS", cell, &addr->transport);
 
 	/* quick scan of the list to see if we already have the server */
 	read_lock(&cell->servers_lock);
 
 	list_for_each_entry(server, &cell->servers, link) {
-		if (server->addr.s_addr == addr->s_addr)
+		if (memcmp(&server->addr, addr, sizeof(*addr)) == 0)
 			goto found_server_quickly;
 	}
 	read_unlock(&cell->servers_lock);

@@ -117,7 +118,7 @@ struct afs_server *afs_lookup_server(struct afs_cell *cell,
 
 	/* check the cell's server list again */
 	list_for_each_entry(server, &cell->servers, link) {
-		if (server->addr.s_addr == addr->s_addr)
+		if (memcmp(&server->addr, addr, sizeof(*addr)) == 0)
 			goto found_server;
 	}

@@ -173,9 +174,9 @@ struct afs_server *afs_find_server(struct afs_net *net,
 {
 	struct afs_server *server = NULL;
 	struct rb_node *p;
-	struct in_addr addr = srx->transport.sin.sin_addr;
+	int diff;
 
-	_enter("{%d,%pI4}", srx->transport.family, &addr.s_addr);
+	_enter("{%d,%pIS}", srx->transport.family, &srx->transport);
 
 	if (srx->transport.family != AF_INET) {
 		WARN(true, "AFS does not yes support non-IPv4 addresses\n");

@@ -190,9 +191,10 @@ struct afs_server *afs_find_server(struct afs_net *net,
 
 		_debug("- consider %p", p);
 
-		if (addr.s_addr < server->addr.s_addr) {
+		diff = memcmp(srx, &server->addr, sizeof(*srx));
+		if (diff < 0) {
 			p = p->rb_left;
-		} else if (addr.s_addr > server->addr.s_addr) {
+		} else if (diff > 0) {
 			p = p->rb_right;
 		} else {
 			afs_get_server(server);

@@ -203,7 +205,6 @@ struct afs_server *afs_find_server(struct afs_net *net,
 	server = NULL;
 found:
 	read_unlock(&net->servers_lock);
-	ASSERTIFCMP(server, server->addr.s_addr, ==, addr.s_addr);
 	_leave(" = %p", server);
 	return server;
 }

fs/afs/vlclient.c +12 -8

index f5a043a9ba61..48d137628d6a 100644
@@ -12,6 +12,7 @@
 #include <linux/gfp.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include "afs_fs.h"
 #include "internal.h"
 
 /*

@@ -83,8 +84,15 @@ static int afs_deliver_vl_get_entry_by_xxx(struct afs_call *call)
 	bp++; /* type */
 	entry->nservers = ntohl(*bp++);
 
-	for (loop = 0; loop < 8; loop++)
-		entry->servers[loop].s_addr = *bp++;
+	for (loop = 0; loop < 8; loop++) {
+		entry->servers[loop].srx_family = AF_RXRPC;
+		entry->servers[loop].srx_service = FS_SERVICE;
+		entry->servers[loop].transport_type = SOCK_DGRAM;
+		entry->servers[loop].transport_len = sizeof(entry->servers[loop].transport.sin);
+		entry->servers[loop].transport.sin.sin_family = AF_INET;
+		entry->servers[loop].transport.sin.sin_port = htons(AFS_FS_PORT);
+		entry->servers[loop].transport.sin.sin_addr.s_addr = *bp++;
+	}
 
 	bp += 8; /* partition IDs */

@@ -144,7 +152,7 @@ static const struct afs_call_type afs_RXVLGetEntryById = {
  * dispatch a get volume entry by name operation
  */
 int afs_vl_get_entry_by_name(struct afs_net *net,
-			     struct in_addr *addr,
+			     struct sockaddr_rxrpc *addr,
 			     struct key *key,
 			     const char *volname,
 			     struct afs_cache_vlocation *entry,

@@ -166,8 +174,6 @@ int afs_vl_get_entry_by_name(struct afs_net *net,
 
 	call->key = key;
 	call->reply = entry;
-	call->service_id = VL_SERVICE;
-	call->port = htons(AFS_VL_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

@@ -185,7 +191,7 @@ int afs_vl_get_entry_by_name(struct afs_net *net,
  * dispatch a get volume entry by ID operation
  */
 int afs_vl_get_entry_by_id(struct afs_net *net,
-			   struct in_addr *addr,
+			   struct sockaddr_rxrpc *addr,
 			   struct key *key,
 			   afs_volid_t volid,
 			   afs_voltype_t voltype,

@@ -203,8 +209,6 @@ int afs_vl_get_entry_by_id(struct afs_net *net,
 
 	call->key = key;
 	call->reply = entry;
-	call->service_id = VL_SERVICE;
-	call->port = htons(AFS_VL_PORT);
 
 	/* marshall the parameters */
 	bp = call->request;

fs/afs/vlocation.c +8 ~~-22~~

index d9a5d7acdb86..4f8c15c09a6d 100644
@@ -30,7 +30,6 @@ static int afs_vlocation_access_vl_by_name(struct afs_vlocation *vl,
 					   struct afs_cache_vlocation *vldb)
 {
 	struct afs_cell *cell = vl->cell;
-	struct in_addr addr;
 	int count, ret;
 
 	_enter("%s,%s", cell->name, vl->vldb.name);

@@ -38,12 +37,12 @@ static int afs_vlocation_access_vl_by_name(struct afs_vlocation *vl,
 	down_write(&vl->cell->vl_sem);
 	ret = -ENOMEDIUM;
 	for (count = cell->vl_naddrs; count > 0; count--) {
-		addr = cell->vl_addrs[cell->vl_curr_svix];
+		struct sockaddr_rxrpc *addr = &cell->vl_addrs[cell->vl_curr_svix];
 
-		_debug("CellServ[%hu]: %08x", cell->vl_curr_svix, addr.s_addr);
+		_debug("CellServ[%hu]: %pIS", cell->vl_curr_svix, &addr->transport);
 
 		/* attempt to access the VL server */
-		ret = afs_vl_get_entry_by_name(cell->net, &addr, key,
+		ret = afs_vl_get_entry_by_name(cell->net, addr, key,
 					       vl->vldb.name, vldb, false);
 		switch (ret) {
 		case 0:

@@ -88,7 +87,6 @@ static int afs_vlocation_access_vl_by_id(struct afs_vlocation *vl,
 					 struct afs_cache_vlocation *vldb)
 {
 	struct afs_cell *cell = vl->cell;
-	struct in_addr addr;
 	int count, ret;
 
 	_enter("%s,%x,%d,", cell->name, volid, voltype);

@@ -96,12 +94,12 @@ static int afs_vlocation_access_vl_by_id(struct afs_vlocation *vl,
 	down_write(&vl->cell->vl_sem);
 	ret = -ENOMEDIUM;
 	for (count = cell->vl_naddrs; count > 0; count--) {
-		addr = cell->vl_addrs[cell->vl_curr_svix];
+		struct sockaddr_rxrpc *addr = &cell->vl_addrs[cell->vl_curr_svix];
 
-		_debug("CellServ[%hu]: %08x", cell->vl_curr_svix, addr.s_addr);
+		_debug("CellServ[%hu]: %pIS", cell->vl_curr_svix, &addr->transport);
 
 		/* attempt to access the VL server */
-		ret = afs_vl_get_entry_by_id(cell->net, &addr, key, volid,
+		ret = afs_vl_get_entry_by_id(cell->net, addr, key, volid,
 					     voltype, vldb, false);
 		switch (ret) {
 		case 0:

@@ -192,15 +190,7 @@ static int afs_vlocation_update_record(struct afs_vlocation *vl,
 	int ret;
 
 	/* try to look up a cached volume in the cell VL databases by ID */
-	_debug("Locally Cached: %s %02x { %08x(%x) %08x(%x) %08x(%x) }",
-	       vl->vldb.name,
-	       vl->vldb.vidmask,
-	       ntohl(vl->vldb.servers[0].s_addr),
-	       vl->vldb.srvtmask[0],
-	       ntohl(vl->vldb.servers[1].s_addr),
-	       vl->vldb.srvtmask[1],
-	       ntohl(vl->vldb.servers[2].s_addr),
-	       vl->vldb.srvtmask[2]);
+	_debug("Locally Cached: %s %02x", vl->vldb.name, vl->vldb.vidmask);
 
 	_debug("Vids: %08x %08x %08x",
 	       vl->vldb.vid[0],

@@ -258,11 +248,7 @@ static int afs_vlocation_update_record(struct afs_vlocation *vl,
 static void afs_vlocation_apply_update(struct afs_vlocation *vl,
 				       struct afs_cache_vlocation *vldb)
 {
-	_debug("Done VL Lookup: %s %02x { %08x(%x) %08x(%x) %08x(%x) }",
-	       vldb->name, vldb->vidmask,
-	       ntohl(vldb->servers[0].s_addr), vldb->srvtmask[0],
-	       ntohl(vldb->servers[1].s_addr), vldb->srvtmask[1],
-	       ntohl(vldb->servers[2].s_addr), vldb->srvtmask[2]);
+	_debug("Done VL Lookup: %s %02x", vldb->name, vldb->vidmask);
 
 	_debug("Vids: %08x %08x %08x",
 	       vldb->vid[0], vldb->vid[1], vldb->vid[2]);

fs/afs/vnode.c +14 ~~-14~~

index dcb956143c86..64834b20f0f6 100644
@@ -354,8 +354,8 @@ int afs_vnode_fetch_status(struct afs_vnode *vnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %p{%08x}",
-		       server, ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %p{%pIS}",
+		       server, &server->addr.transport);
 
 		ret = afs_fs_fetch_file_status(server, key, vnode, NULL,
 					       false);

@@ -418,7 +418,7 @@ int afs_vnode_fetch_data(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_fetch_data(server, key, vnode, desc,
 					false);

@@ -474,7 +474,7 @@ int afs_vnode_create(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_create(server, key, vnode, name, mode, newfid,
 				    newstatus, newcb, false);

@@ -530,7 +530,7 @@ int afs_vnode_remove(struct afs_vnode *vnode, struct key *key, const char *name,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_remove(server, key, vnode, name, isdir,
 				    false);

@@ -592,7 +592,7 @@ int afs_vnode_link(struct afs_vnode *dvnode, struct afs_vnode *vnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_link(server, key, dvnode, vnode, name,
 				  false);

@@ -656,7 +656,7 @@ int afs_vnode_symlink(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_symlink(server, key, vnode, name, content,
 				     newfid, newstatus, false);

@@ -726,7 +726,7 @@ int afs_vnode_rename(struct afs_vnode *orig_dvnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_rename(server, key, orig_dvnode, orig_name,
 				    new_dvnode, new_name, false);

@@ -792,7 +792,7 @@ int afs_vnode_store_data(struct afs_writeback *wb, pgoff_t first, pgoff_t last,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_store_data(server, wb, first, last, offset, to,
 					false);

@@ -845,7 +845,7 @@ int afs_vnode_setattr(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_setattr(server, key, vnode, attr, false);

@@ -892,7 +892,7 @@ int afs_vnode_get_volume_status(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_get_volume_status(server, key, vnode, vs, false);

@@ -931,7 +931,7 @@ int afs_vnode_set_lock(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_set_lock(server, key, vnode, type, false);

@@ -969,7 +969,7 @@ int afs_vnode_extend_lock(struct afs_vnode *vnode, struct key *key)
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_extend_lock(server, key, vnode, false);

@@ -1007,7 +1007,7 @@ int afs_vnode_release_lock(struct afs_vnode *vnode, struct key *key)
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %08x\n", ntohl(server->addr.s_addr));
+		_debug("USING SERVER: %pIS\n", &server->addr.transport);
 
 		ret = afs_fs_release_lock(server, key, vnode, false);

fs/afs/volume.c +4 -5

index bc52203695b7..fbbb470ac027 100644
@@ -248,8 +248,8 @@ struct afs_server *afs_volume_pick_fileserver(struct afs_vnode *vnode)
 		case 0:
 			afs_get_server(server);
 			up_read(&volume->server_sem);
-			_leave(" = %p (picked %08x)",
-			       server, ntohl(server->addr.s_addr));
+			_leave(" = %p (picked %pIS)",
+			       server, &server->addr.transport);
 			return server;
 
 		case -ENETUNREACH:

@@ -303,9 +303,8 @@ int afs_volume_release_fileserver(struct afs_vnode *vnode,
 	struct afs_volume *volume = vnode->volume;
 	unsigned loop;
 
-	_enter("%s,%08x,%d",
-	       volume->vlocation->vldb.name, ntohl(server->addr.s_addr),
-	       result);
+	_enter("%s,%pIS,%d",
+	       volume->vlocation->vldb.name, &server->addr.transport, result);
 
 	switch (result) {
 		/* success */

David Howells

author maintainer

2017-09-01 15:41 UTC #19 lore raw

RFC PATCH 9/11 afs: Allow IPv6 address specification of VL servers

In reply to David Howells

Allow VL server specifications to be given IPv6 addresses as well as IPv4 addresses, for example as:

	echo add procyon.org.uk 1111:2222:3333:0:4444:5555:6666:7777 >/proc/fs/afs/cells

Note that ':' is the expected separator for separating IPv4 addresses, but if a ',' is detected or no '.' is detected in the string, the delimiter is switched to ','.

This also works with DNS AFSDB or SRV record strings fetched by upcall from userspace.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/cell.c     |   31 +++++++++++++++++++++----------
 fs/afs/proc.c     |    2 +-
 fs/afs/rxrpc.c    |   11 +++++------
 fs/afs/server.c   |    5 -----
 fs/afs/vlclient.c |   13 +++++++++----
 5 files changed, 36 insertions(+), 26 deletions(-)

fs/afs/cell.c +21 ~~-10~~

index 3a6c91ae11a3..6c49532a79de 100644
@@ -15,6 +15,7 @@
 #include <linux/ctype.h>
 #include <linux/dns_resolver.h>
 #include <linux/sched.h>
+#include <linux/inet.h>
 #include <keys/rxrpc-type.h>
 #include "internal.h"

@@ -86,28 +87,38 @@ static struct afs_cell *afs_cell_alloc(struct afs_net *net,
 		delimiter = ',';
 
 	} else {
+		if (strchr(vllist, ',') || !strchr(vllist, '.'))
+			delimiter = ',';
 		_vllist = vllist;
 	}
 
 	/* fill in the VL server list from the rest of the string */
 	do {
 		struct sockaddr_rxrpc *srx = &cell->vl_addrs[cell->vl_naddrs];
-		unsigned a, b, c, d;
+		const char *end;
 
 		next = strchr(_vllist, delimiter);
 		if (next)
 			*next++ = 0;
 
-		if (sscanf(_vllist, "%u.%u.%u.%u", &a, &b, &c, &d) != 4)
-			goto bad_address;
-
-		if (a > 255 || b > 255 || c > 255 || d > 255)
+		if (in4_pton(_vllist, -1, (u8 *)&srx->transport.sin6.sin6_addr.s6_addr32[3],
+			     -1, &end)) {
+			srx->transport_len		= sizeof(struct sockaddr_in6);
+			srx->transport.sin6.sin6_family	= AF_INET6;
+			srx->transport.sin6.sin6_flowinfo = 0;
+			srx->transport.sin6.sin6_scope_id = 0;
+			srx->transport.sin6.sin6_addr.s6_addr32[0] = 0;
+			srx->transport.sin6.sin6_addr.s6_addr32[1] = 0;
+			srx->transport.sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
+		} else if (in6_pton(_vllist, -1, srx->transport.sin6.sin6_addr.s6_addr,
+				    -1, &end)) {
+			srx->transport_len		= sizeof(struct sockaddr_in6);
+			srx->transport.sin6.sin6_family	= AF_INET6;
+			srx->transport.sin6.sin6_flowinfo = 0;
+			srx->transport.sin6.sin6_scope_id = 0;
+		} else {
 			goto bad_address;
-
-		srx->transport_len		= sizeof(struct sockaddr_in);
-		srx->transport.sin.sin_family	= AF_INET;
-		srx->transport.sin.sin_addr.s_addr =
-			htonl((a << 24) | (b << 16) | (c << 8) | d);
+		}
 
 	} while (cell->vl_naddrs++,
 		 cell->vl_naddrs < AFS_CELL_MAX_ADDRS && (_vllist = next));

fs/afs/proc.c +1 -1

index 4d609869a57b..ee10f0089d87 100644
@@ -662,7 +662,7 @@ static int afs_proc_cell_servers_show(struct seq_file *m, void *v)
 
 	/* display one cell per line on subsequent lines */
 	sprintf(ipaddr, "%pISp", &server->addr.transport);
-	seq_printf(m, "%3d %-15.15s %5d\n",
+	seq_printf(m, "%3d %-15s %5d\n",
 		   atomic_read(&server->usage), ipaddr, server->fs_state);
 
 	return 0;

fs/afs/rxrpc.c +5 -6

index f57a2d41a310..805ae0542478 100644
@@ -46,21 +46,20 @@ int afs_open_socket(struct afs_net *net)
 
 	_enter("");
 
-	ret = sock_create_kern(&init_net, AF_RXRPC, SOCK_DGRAM, PF_INET, &socket);
+	ret = sock_create_kern(&init_net, AF_RXRPC, SOCK_DGRAM, PF_INET6, &socket);
 	if (ret < 0)
 		goto error_1;
 
 	socket->sk->sk_allocation = GFP_NOFS;
 
 	/* bind the callback manager's address to make this a server socket */
+	memset(&srx, 0, sizeof(srx));
 	srx.srx_family			= AF_RXRPC;
 	srx.srx_service			= CM_SERVICE;
 	srx.transport_type		= SOCK_DGRAM;
-	srx.transport_len		= sizeof(srx.transport.sin);
-	srx.transport.sin.sin_family	= AF_INET;
-	srx.transport.sin.sin_port	= htons(AFS_CM_PORT);
-	memset(&srx.transport.sin.sin_addr, 0,
-	       sizeof(srx.transport.sin.sin_addr));
+	srx.transport_len		= sizeof(srx.transport.sin6);
+	srx.transport.sin6.sin6_family	= AF_INET6;
+	srx.transport.sin6.sin6_port	= htons(AFS_CM_PORT);
 
 	ret = kernel_bind(socket, (struct sockaddr *) &srx, sizeof(srx));
 	if (ret < 0)

fs/afs/server.c +0 -5

index 7d103321efab..9b38f386a142 100644
@@ -178,11 +178,6 @@ struct afs_server *afs_find_server(struct afs_net *net,
 
 	_enter("{%d,%pIS}", srx->transport.family, &srx->transport);
 
-	if (srx->transport.family != AF_INET) {
-		WARN(true, "AFS does not yes support non-IPv4 addresses\n");
-		return NULL;
-	}
-
 	read_lock(&net->servers_lock);
 
 	p = net->servers.rb_node;

fs/afs/vlclient.c +9 -4

index 48d137628d6a..276319aa86d8 100644
@@ -88,10 +88,15 @@ static int afs_deliver_vl_get_entry_by_xxx(struct afs_call *call)
 		entry->servers[loop].srx_family = AF_RXRPC;
 		entry->servers[loop].srx_service = FS_SERVICE;
 		entry->servers[loop].transport_type = SOCK_DGRAM;
-		entry->servers[loop].transport_len = sizeof(entry->servers[loop].transport.sin);
-		entry->servers[loop].transport.sin.sin_family = AF_INET;
-		entry->servers[loop].transport.sin.sin_port = htons(AFS_FS_PORT);
-		entry->servers[loop].transport.sin.sin_addr.s_addr = *bp++;
+		entry->servers[loop].transport_len = sizeof(entry->servers[loop].transport.sin6);
+		entry->servers[loop].transport.sin6.sin6_family = AF_INET6;
+		entry->servers[loop].transport.sin6.sin6_port = htons(AFS_FS_PORT);
+		entry->servers[loop].transport.sin6.sin6_flowinfo = 0;
+		entry->servers[loop].transport.sin6.sin6_scope_id = 0;
+		entry->servers[loop].transport.sin6.sin6_addr.s6_addr32[0] = 0;
+		entry->servers[loop].transport.sin6.sin6_addr.s6_addr32[1] = 0;
+		entry->servers[loop].transport.sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
+		entry->servers[loop].transport.sin6.sin6_addr.s6_addr32[3] = *bp++;
 	}
 
 	bp += 8; /* partition IDs */

David Howells

author maintainer

2017-09-01 15:42 UTC #20 lore raw

RFC PATCH 10/11 afs: Overhaul cell database management

In reply to David Howells

Overhaul the way that the in-kernel AFS client keeps track of cells in the following manner:

(1) Cells are now held in an rbtree to make walking them quicker and RCU managed (though this is probably overkill).

(2) Cells now have a manager work item that:

(A) Looks after fetching and refreshing the VL server list.

(B) Manages cell record lifetime, including initialising and destruction.

(B) Manages cell record caching whereby threads are kept around for a certain time after last use and then destroyed.

(C) Manages the FS-Cache index cookie for a cell. It is not permitted for a cookie to be in use twice, so we have to be careful to not allow a new cell record to exist at the same time as an old record of the same name.

(3) Each AFS network namespace is given a manager work item that manages the cells within it, maintaining a single timer to prod cells into updating their DNS records.

This uses the reduce_timer() facility to make the timer expire at the soonest timed event that needs happening.

(4) When a module is being unloaded, cells and cell managers are now counted out using dec_after_work() to make sure the module text is pinned until after the data structures have been cleaned up.

(5) Each cell's VL server list is now protected by a seqlock rather than a semaphore.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/callback.c  |    2 
 fs/afs/cell.c      |  913 ++++++++++++++++++++++++++++++++++++----------------
 fs/afs/internal.h  |   62 +++-
 fs/afs/main.c      |   16 +
 fs/afs/proc.c      |   15 -
 fs/afs/server.c    |    3 
 fs/afs/super.c     |   20 +
 fs/afs/vlocation.c |    6 
 fs/afs/xattr.c     |    2 
 9 files changed, 710 insertions(+), 329 deletions(-)

fs/afs/callback.c +1 -1

index d12dffb76b67..bdccc4e72c0e 100644
@@ -341,7 +341,7 @@ void afs_dispatch_give_up_callbacks(struct work_struct *work)
 	 *   had callbacks entirely, and the server will call us later to break
 	 *   them
 	 */
-	afs_fs_give_up_callbacks(server->cell->net, server, true);
+	afs_fs_give_up_callbacks(server->net, server, true);
 }
 
 /*

fs/afs/cell.c +629 ~~-284~~

1,088 lines not on this page · raw message

fs/afs/internal.h +46 ~~-16~~

103 lines not on this page · raw message

fs/afs/main.c +11 -5

index 87b1a9c8000d..e0342e445099 100644
@@ -46,12 +46,15 @@ static int __net_init afs_net_init(struct afs_net *net)
 
 	INIT_WORK(&net->charge_preallocation_work, afs_charge_preallocation);
 	mutex_init(&net->socket_mutex);
-	INIT_LIST_HEAD(&net->cells);
-	rwlock_init(&net->cells_lock);
-	init_rwsem(&net->cells_sem);
-	init_waitqueue_head(&net->cells_freeable_wq);
-	init_rwsem(&net->proc_cells_sem);
+
+	net->cells = RB_ROOT;
+	seqlock_init(&net->cells_lock);
+	INIT_WORK(&net->cells_manager, afs_manage_cells);
+	setup_timer(&net->cells_timer, afs_cells_timer, (unsigned long)net);
+
+	spin_lock_init(&net->proc_cells_lock);
 	INIT_LIST_HEAD(&net->proc_cells);
+
 	INIT_LIST_HEAD(&net->vl_updates);
 	INIT_LIST_HEAD(&net->vl_graveyard);
 	INIT_DELAYED_WORK(&net->vl_reaper, afs_vlocation_reaper);

@@ -82,11 +85,14 @@ static int __net_init afs_net_init(struct afs_net *net)
 	return 0;
 
 error_open_socket:
+	net->live = false;
 	afs_vlocation_purge(net);
 	afs_cell_purge(net);
 error_cell_init:
+	net->live = false;
 	afs_proc_cleanup(net);
 error_proc:
+	net->live = false;
 	return ret;
 }

fs/afs/proc.c +6 -9

index ee10f0089d87..df3614306056 100644
@@ -186,7 +186,7 @@ static void *afs_proc_cells_start(struct seq_file *m, loff_t *_pos)
 {
 	struct afs_net *net = afs_seq2net(m);
 
-	down_read(&net->proc_cells_sem);
+	rcu_read_lock();
 	return seq_list_start_head(&net->proc_cells, *_pos);
 }

@@ -205,9 +205,7 @@ static void *afs_proc_cells_next(struct seq_file *m, void *v, loff_t *pos)
  */
 static void afs_proc_cells_stop(struct seq_file *m, void *v)
 {
-	struct afs_net *net = afs_seq2net(m);
-
-	up_read(&net->proc_cells_sem);
+	rcu_read_unlock();
 }
 
 /*

@@ -225,8 +223,7 @@ static int afs_proc_cells_show(struct seq_file *m, void *v)
 	}
 
 	/* display one cell per line on subsequent lines */
-	seq_printf(m, "%3d %s\n",
-		   atomic_read(&cell->usage), cell->name);
+	seq_printf(m, "%3u %s\n", refcount_read(&cell->usage), cell->name);
 	return 0;
 }

@@ -279,13 +276,13 @@ static ssize_t afs_proc_cells_write(struct file *file, const char __user *buf,
 	if (strcmp(kbuf, "add") == 0) {
 		struct afs_cell *cell;
 
-		cell = afs_cell_create(net, name, strlen(name), args, false);
+		cell = afs_lookup_cell(net, name, strlen(name), args, true);
 		if (IS_ERR(cell)) {
 			ret = PTR_ERR(cell);
 			goto done;
 		}
 
-		afs_put_cell(cell);
+		set_bit(AFS_CELL_FL_NO_GC, &cell->flags);
 		printk("kAFS: Added new cell '%s'\n", name);
 	} else {
 		goto inval;

@@ -354,7 +351,7 @@ int afs_proc_cell_setup(struct afs_net *net, struct afs_cell *cell)
 {
 	struct proc_dir_entry *dir;
 
-	_enter("%p{%s}", cell, cell->name);
+	_enter("%p{%s},%p", cell, cell->name, net->proc_afs);
 
 	dir = proc_mkdir(cell->name, net->proc_afs);
 	if (!dir)

fs/afs/server.c +2 -1

index 9b38f386a142..57c2f605e11b 100644
@@ -68,6 +68,7 @@ static struct afs_server *afs_alloc_server(struct afs_cell *cell,
 	server = kzalloc(sizeof(struct afs_server), GFP_KERNEL);
 	if (server) {
 		atomic_set(&server->usage, 1);
+		server->net = cell->net;
 		server->cell = cell;
 
 		INIT_LIST_HEAD(&server->link);

@@ -254,7 +255,7 @@ static void afs_destroy_server(struct afs_server *server)
 	ASSERTCMP(server->cb_break_head, ==, server->cb_break_tail);
 	ASSERTCMP(atomic_read(&server->cb_break_n), ==, 0);
 
-	afs_put_cell(server->cell);
+	afs_put_cell(server->net, server->cell);
 	kfree(server);
 }

fs/afs/super.c +11 -9

index 1bfc7b28700b..5113a48f907d 100644
@@ -200,13 +200,14 @@ static int afs_parse_options(struct afs_mount_params *params,
 		token = match_token(p, afs_options_list, args);
 		switch (token) {
 		case afs_opt_cell:
-			cell = afs_cell_lookup(params->net,
-					       args[0].from,
-					       args[0].to - args[0].from,
-					       false);
+			rcu_read_lock();
+			cell = afs_lookup_cell_rcu(params->net,
+						   args[0].from,
+						   args[0].to - args[0].from);
+			rcu_read_unlock();
 			if (IS_ERR(cell))
 				return PTR_ERR(cell);
-			afs_put_cell(params->cell);
+			afs_put_cell(params->net, params->cell);
 			params->cell = cell;
 			break;

@@ -308,13 +309,14 @@ static int afs_parse_device_name(struct afs_mount_params *params,
 
 	/* lookup the cell record */
 	if (cellname || !params->cell) {
-		cell = afs_cell_lookup(params->net, cellname, cellnamesz, true);
+		cell = afs_lookup_cell(params->net, cellname, cellnamesz,
+				       NULL, false);
 		if (IS_ERR(cell)) {
 			printk(KERN_ERR "kAFS: unable to lookup cell '%*.*s'\n",
 			       cellnamesz, cellnamesz, cellname ?: "");
 			return PTR_ERR(cell);
 		}
-		afs_put_cell(params->cell);
+		afs_put_cell(params->net, params->cell);
 		params->cell = cell;
 	}

@@ -474,7 +476,7 @@ static struct dentry *afs_mount(struct file_system_type *fs_type,
 		kfree(as);
 	}
 
-	afs_put_cell(params.cell);
+	afs_put_cell(params.net, params.cell);
 	kfree(new_opts);
 	_leave(" = 0 [%p]", sb);
 	return dget(sb->s_root);

@@ -488,7 +490,7 @@ static struct dentry *afs_mount(struct file_system_type *fs_type,
 error_vol:
 	afs_put_volume(params.net, vol);
 error:
-	afs_put_cell(params.cell);
+	afs_put_cell(params.net, params.cell);
 	key_put(params.key);
 	kfree(new_opts);
 	_leave(" = %d", ret);

fs/afs/vlocation.c +3 -3

index 4f8c15c09a6d..ec5ab8dc9bc8 100644
@@ -480,11 +480,11 @@ void afs_put_vlocation(struct afs_net *net, struct afs_vlocation *vl)
 /*
  * destroy a dead volume location record
  */
-static void afs_vlocation_destroy(struct afs_vlocation *vl)
+static void afs_vlocation_destroy(struct afs_net *net, struct afs_vlocation *vl)
 {
 	_enter("%p", vl);
 
-	afs_put_cell(vl->cell);
+	afs_put_cell(net, vl->cell);
 	kfree(vl);
 }

@@ -539,7 +539,7 @@ void afs_vlocation_reaper(struct work_struct *work)
 	while (!list_empty(&corpses)) {
 		vl = list_entry(corpses.next, struct afs_vlocation, grave);
 		list_del(&vl->grave);
-		afs_vlocation_destroy(vl);
+		afs_vlocation_destroy(net, vl);
 	}
 
 	_leave("");

fs/afs/xattr.c +1 -1

index 2830e4f48d85..e58e00ee9747 100644
@@ -45,7 +45,7 @@ static int afs_xattr_get_cell(const struct xattr_handler *handler,
 	struct afs_cell *cell = vnode->volume->cell;
 	size_t namelen;
 
-	namelen = strlen(cell->name);
+	namelen = cell->name_len;
 	if (size == 0)
 		return namelen;
 	if (namelen > size)

David Howells

author maintainer

2017-09-01 15:42 UTC #21 lore raw

RFC PATCH 11/11 afs: Retry rxrpc calls with address rotation on network error

In reply to David Howells

When a network error occurs when we attempt a call, we want to rotate the set of addresses we have for that peer and try the call again. Use the new AF_RXRPC call-retrying facility to do this, thereby avoiding the need to re-encrypt each time as this allows us to reuse the Tx-queue from the dead call.

This method will work for accessing alternate VL servers and the various addresses available for a single FS server, but should not be used to go to alternate FS servers since that has other implications (such as getting callbacks on other servers).

To this end:

(1) An 'address list' concept is introduced. Address lists are RCU replaceable lists of addresses.

(2) A cell's VL server address list can be loaded directly via insmod or echo to /proc/fs/afs/cells or dynamically from a DNS query for AFSDB or SRV records.

(3) An FS server's address list, for the moment, has a single entry that is the key to the server list. This will change in the future when a server is instead keyed on its UUID and the VL.GetAddrsU operation is used.

(4) Anyone wanting to use a cell's VL server address must wait until the cell record comes online and has tried to obtain some addresses.

(5) An 'address cursor' concept is introduced to handle stepping over the address list. For client calls, this is driven from a wrapper around rxrpc_kernel_send_data(). It isn't used for CM service call replies as they have to go to the caller's address.

In the future, we might want to annotate the list with information about how each address fares. We might then want to propagate such annotations over address list replacement.

Whilst we're at it, we allow IPv6 addresses to be specified in colon-delimited lists by enclosing them in square brackets.

Signed-off-by David Howells <[email protected]>

---

 fs/afs/Makefile    |    1 
 fs/afs/addr_list.c |  310 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/afs/cell.c      |  191 ++++++++++++--------------------
 fs/afs/fsclient.c  |   54 ++++++---
 fs/afs/internal.h  |   60 ++++++++--
 fs/afs/proc.c      |   23 ++--
 fs/afs/rxrpc.c     |  180 +++++++++++++++++++++++++++---
 fs/afs/server.c    |   66 +++++++----
 fs/afs/vlclient.c  |   19 ++-
 fs/afs/vlocation.c |  150 +++----------------------
 fs/afs/vnode.c     |   27 ++---
 fs/afs/volume.c    |    5 +
 12 files changed, 720 insertions(+), 366 deletions(-)
 create mode 100644 fs/afs/addr_list.c

fs/afs/Makefile +1 -0

index 095c54165dfd..7cb4d55f6f1f 100644
@@ -6,6 +6,7 @@ afs-cache-$(CONFIG_AFS_FSCACHE) := cache.o
 
 kafs-objs := \
 	$(afs-cache-y) \
+	addr_list.o \
 	callback.o \
 	cell.o \
 	cmservice.o \

fs/afs/addr_list.c new file +310 -0

310 lines not on this page · raw message

fs/afs/cell.c +70 ~~-121~~

285 lines not on this page · raw message

fs/afs/fsclient.c +36 ~~-18~~

160 lines not on this page · raw message

fs/afs/internal.h +47 ~~-13~~

124 lines not on this page · raw message

fs/afs/proc.c +11 ~~-12~~

index df3614306056..d75626613ed9 100644
@@ -514,23 +514,23 @@ static int afs_proc_cell_vlservers_open(struct inode *inode, struct file *file)
  */
 static void *afs_proc_cell_vlservers_start(struct seq_file *m, loff_t *_pos)
 {
+	struct afs_addr_list *alist;
 	struct afs_cell *cell = m->private;
 	loff_t pos = *_pos;
 
-	_enter("cell=%p pos=%Ld", cell, *_pos);
+	rcu_read_lock();
 
-	/* lock the list against modification */
-	down_read(&cell->vl_sem);
+	alist = rcu_dereference(cell->vl_addrs);
 
 	/* allow for the header line */
 	if (!pos)
 		return (void *) 1;
 	pos--;
 
-	if (pos >= cell->vl_naddrs)
+	if (!alist || pos >= alist->nr_addrs)
 		return NULL;
 
-	return &cell->vl_addrs[pos];
+	return alist->addrs + pos;
 }
 
 /*

@@ -539,17 +539,18 @@ static void *afs_proc_cell_vlservers_start(struct seq_file *m, loff_t *_pos)
 static void *afs_proc_cell_vlservers_next(struct seq_file *p, void *v,
 					  loff_t *_pos)
 {
+	struct afs_addr_list *alist;
 	struct afs_cell *cell = p->private;
 	loff_t pos;
 
-	_enter("cell=%p{nad=%u} pos=%Ld", cell, cell->vl_naddrs, *_pos);
+	alist = rcu_dereference(cell->vl_addrs);
 
 	pos = *_pos;
 	(*_pos)++;
-	if (pos >= cell->vl_naddrs)
+	if (!alist || pos >= alist->nr_addrs)
 		return NULL;
 
-	return &cell->vl_addrs[pos];
+	return alist->addrs + pos;
 }
 
 /*

@@ -557,9 +558,7 @@ static void *afs_proc_cell_vlservers_next(struct seq_file *p, void *v,
  */
 static void afs_proc_cell_vlservers_stop(struct seq_file *p, void *v)
 {
-	struct afs_cell *cell = p->private;
-
-	up_read(&cell->vl_sem);
+	rcu_read_unlock();
 }
 
 /*

@@ -658,7 +657,7 @@ static int afs_proc_cell_servers_show(struct seq_file *m, void *v)
 	}
 
 	/* display one cell per line on subsequent lines */
-	sprintf(ipaddr, "%pISp", &server->addr.transport);
+	sprintf(ipaddr, "%pISp", &server->addrs->addrs[0].transport);
 	seq_printf(m, "%3d %-15s %5d\n",
 		   atomic_read(&server->usage), ipaddr, server->fs_state);

fs/afs/rxrpc.c +160 ~~-20~~

248 lines not on this page · raw message

fs/afs/server.c +41 ~~-25~~

103 lines not on this page · raw message

fs/afs/vlclient.c +11 -8

index 276319aa86d8..54d02e5ea20a 100644
@@ -156,8 +156,7 @@ static const struct afs_call_type afs_RXVLGetEntryById = {
 /*
  * dispatch a get volume entry by name operation
  */
-int afs_vl_get_entry_by_name(struct afs_net *net,
-			     struct sockaddr_rxrpc *addr,
+int afs_vl_get_entry_by_name(struct afs_cell *cell,
 			     struct key *key,
 			     const char *volname,
 			     struct afs_cache_vlocation *entry,

@@ -173,10 +172,13 @@ int afs_vl_get_entry_by_name(struct afs_net *net,
 	padsz = (4 - (volnamesz & 3)) & 3;
 	reqsz = 8 + volnamesz + padsz;
 
-	call = afs_alloc_flat_call(net, &afs_RXVLGetEntryByName, reqsz, 384);
+	call = afs_alloc_flat_call(cell->net, &afs_RXVLGetEntryByName,
+				   reqsz, 384);
 	if (!call)
 		return -ENOMEM;
 
+	afs_set_vl_cursor(call, cell);
+
 	call->key = key;
 	call->reply = entry;

@@ -189,14 +191,13 @@ int afs_vl_get_entry_by_name(struct afs_net *net,
 		memset((void *) bp + volnamesz, 0, padsz);
 
 	/* initiate the call */
-	return afs_make_call(addr, call, GFP_KERNEL, async);
+	return afs_make_call(call, GFP_KERNEL, async);
 }
 
 /*
  * dispatch a get volume entry by ID operation
  */
-int afs_vl_get_entry_by_id(struct afs_net *net,
-			   struct sockaddr_rxrpc *addr,
+int afs_vl_get_entry_by_id(struct afs_cell *cell,
 			   struct key *key,
 			   afs_volid_t volid,
 			   afs_voltype_t voltype,

@@ -208,10 +209,12 @@ int afs_vl_get_entry_by_id(struct afs_net *net,
 
 	_enter("");
 
-	call = afs_alloc_flat_call(net, &afs_RXVLGetEntryById, 12, 384);
+	call = afs_alloc_flat_call(cell->net, &afs_RXVLGetEntryById, 12, 384);
 	if (!call)
 		return -ENOMEM;
 
+	afs_set_vl_cursor(call, cell);
+
 	call->key = key;
 	call->reply = entry;

@@ -222,5 +225,5 @@ int afs_vl_get_entry_by_id(struct afs_net *net,
 	*bp   = htonl(voltype);
 
 	/* initiate the call */
-	return afs_make_call(addr, call, GFP_KERNEL, async);
+	return afs_make_call(call, GFP_KERNEL, async);
 }

fs/afs/vlocation.c +17 ~~-133~~

180 lines not on this page · raw message

fs/afs/vnode.c +13 ~~-14~~

index 64834b20f0f6..8dcf4921340a 100644
@@ -354,8 +354,7 @@ int afs_vnode_fetch_status(struct afs_vnode *vnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %p{%pIS}",
-		       server, &server->addr.transport);
+		_debug("USING SERVER: %pISp", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_fetch_file_status(server, key, vnode, NULL,
 					       false);

@@ -418,7 +417,7 @@ int afs_vnode_fetch_data(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pISp", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_fetch_data(server, key, vnode, desc,
 					false);

@@ -474,7 +473,7 @@ int afs_vnode_create(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pISp", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_create(server, key, vnode, name, mode, newfid,
 				    newstatus, newcb, false);

@@ -530,7 +529,7 @@ int afs_vnode_remove(struct afs_vnode *vnode, struct key *key, const char *name,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_remove(server, key, vnode, name, isdir,
 				    false);

@@ -592,7 +591,7 @@ int afs_vnode_link(struct afs_vnode *dvnode, struct afs_vnode *vnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_link(server, key, dvnode, vnode, name,
 				  false);

@@ -656,7 +655,7 @@ int afs_vnode_symlink(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_symlink(server, key, vnode, name, content,
 				     newfid, newstatus, false);

@@ -726,7 +725,7 @@ int afs_vnode_rename(struct afs_vnode *orig_dvnode,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_rename(server, key, orig_dvnode, orig_name,
 				    new_dvnode, new_name, false);

@@ -792,7 +791,7 @@ int afs_vnode_store_data(struct afs_writeback *wb, pgoff_t first, pgoff_t last,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_store_data(server, wb, first, last, offset, to,
 					false);

@@ -845,7 +844,7 @@ int afs_vnode_setattr(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_setattr(server, key, vnode, attr, false);

@@ -892,7 +891,7 @@ int afs_vnode_get_volume_status(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_get_volume_status(server, key, vnode, vs, false);

@@ -931,7 +930,7 @@ int afs_vnode_set_lock(struct afs_vnode *vnode, struct key *key,
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_set_lock(server, key, vnode, type, false);

@@ -969,7 +968,7 @@ int afs_vnode_extend_lock(struct afs_vnode *vnode, struct key *key)
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_extend_lock(server, key, vnode, false);

@@ -1007,7 +1006,7 @@ int afs_vnode_release_lock(struct afs_vnode *vnode, struct key *key)
 		if (IS_ERR(server))
 			goto no_server;
 
-		_debug("USING SERVER: %pIS\n", &server->addr.transport);
+		_debug("USING SERVER: %pIS\n", &server->addrs->addrs[0].transport);
 
 		ret = afs_fs_release_lock(server, key, vnode, false);

fs/afs/volume.c +3 -2

index fbbb470ac027..c0d4e9725d5e 100644
@@ -249,7 +249,7 @@ struct afs_server *afs_volume_pick_fileserver(struct afs_vnode *vnode)
 			afs_get_server(server);
 			up_read(&volume->server_sem);
 			_leave(" = %p (picked %pIS)",
-			       server, &server->addr.transport);
+			       server, &server->addrs->addrs[0].transport);
 			return server;
 
 		case -ENETUNREACH:

@@ -304,7 +304,8 @@ int afs_volume_release_fileserver(struct afs_vnode *vnode,
 	unsigned loop;
 
 	_enter("%s,%pIS,%d",
-	       volume->vlocation->vldb.name, &server->addr.transport, result);
+	       volume->vlocation->vldb.name, &server->addrs->addrs[0].transport,
+	       result);
 
 	switch (result) {
 		/* success */

David Howells

author maintainer

2017-09-01 15:52 UTC #22 lore raw

cover AFS: Namespacing part 1

In reply to David Howells

Here are some changes to the AFS filesystem that form the first part of network-namespacing and IPv6 enabling the AFS filesystem. AF_RXRPC is already namespaced.

This is built on AF_RXRPC changes tagged with rxrpc-next-20170829 (which is also in net-next).

The AFS changes are:

(1) Create a dummy AFS network namespace and shift a bunch of global things into it and start using it.

(2) Add some more AFS RPC protocol definitions.

(3) Update the cache infrastructure to remove some stuff that is redundant or not actually useful and increment the version.

(4) Keep track of internal addresses in terms of sockaddr_rxrpc structs rather than in_addr structs. This will enable the use of IPv6.

(5) Allow IPv6 addresses for VL servers to be specified. Note that this doesn't help with finding FS servers as that requires a protocol change. Such a protocol extension is available in the AuriStor AFS-compatible server, though I haven't implemented that yet.

(6) Overhaul cell database management to manage them better, making them automatically kept up to date from the DNS server.

(7) Make use of the new AF_RXRPC call-retry to implement address rotation for VL servers and FS servers without the need to re-encrypt client call data.

To make this work, I've added some extensions to the core kernel:

(1) Add a decrement-after-return function for workqueues that allows a work item to ask the workqueue manager to decrement an atomic_t and 'wake it up' if it reaches 0. This is analogous to complete_and_exit() and can be used to protect rmmod against code removal.

(2) Add refcount_inc/dec_return() functions that return the new value of the refcount_t. This makes maintaining a cache easier where you want to schedule timed garbage collection when the refcount reaches 1. It also makes tracing easier as the value is obtained atomically.

(3) Pass the wait mode to wait_on_atomic_t() and provide a default action function. This allows various default actions scattered about the place to be deleted.

(4) Add a function to start or reduce the timeout on a timer if it's already running. This makes it easier to maintain a single timer for multiple events without requiring extra locking to check/modify the timer (the timer has its own lock after all).

The patches can be found here also:

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

David

Tejun Heo

maintainer

2017-09-05 13:29 UTC #23 lore raw

In reply to David Howells

Hello, David.

Quoted 11 lines · David Howells

Add a facility to the workqueue subsystem whereby an atomic_t can be registered by a work function such that the work function dispatcher will decrement the atomic after the work function has returned and then call wake_up_atomic() on it if it reached 0.
This is analogous to complete_and_exit() for kernel threads and is used to avoid a race between notifying that a work item is about to finish and the .text segment from a module being discarded.
The way this is used is that the work function calls:

Quoted 7 lines

	dec_after_work(atomic_t *counter);
to register the counter and then process_one_work() calls it, potentially wakes it and clears the registration.
The reason I've used an atomic_t rather than a completion is that (1) it takes up less space and (2) it can monitor multiple objects.

Given how work items are used, I think this is too inviting to abuses where people build complex event chains through these counters and those chains would be completely opaque. If the goal is protecting .text of a work item, can't we just do that? Can you please describe your use case in more detail? Why can't it be done via the usual "flush from exit"?

Thanks.

Signature · 1 line

tejun

David Howells

author maintainer

2017-09-05 14:50 UTC #24 lore raw

Quoted 5 lines · Tejun Heo

Given how work items are used, I think this is too inviting to abuses where people build complex event chains through these counters and those chains would be completely opaque. If the goal is protecting .text of a work item, can't we just do that? Can you please describe your use case in more detail?

With one of my latest patches to AFS, there's a set of cell records, where each cell has a manager work item that mainains that cell, including refreshing DNS records and excising expired records from the list. Performing the excision in the manager work item makes handling the fscache index cookie easier (you can't have two cookies attached to the same object), amongst other things.

There's also an overseer work item that maintains a single expiry timer for all the cells and queues the per-cell work items to do DNS updates and cell removal.

The reason that the overseer exists is that it makes it easier to do a put on a cell. The cell decrements the cell refcount and then wants to schedule the cell for destruction - but it's no longer permitted to touch the cell. I could use atomic_dec_and_lock(), but that's messy. It's cleaner just to set the timer on the overseer and leave it to that.

However, if someone does rmmod, I have to be able to clean everything up. The overseer timer may be queued or running; the overseer may be queued *and* running and may get queued again by the timer; and each cell's work item may be queued *and* running and may get queued again by the manager.

Quoted 1 line

Why can't it be done via the usual "flush from exit"?

Well, it can, but you need a flush for each separate level of dependencies, where one dependency will kick off another level of dependency during the cleanup.

So what I think I would have to do is set a flag to say that no one is allowed to set the timer now (this shouldn't happen outside of server or volume cache clearance), delete the timer synchronously, flush the work queue four times and then do an RCU barrier.

However, since I have volumes with dependencies on servers and cells, possibly with their own managers, I think I may need up to 12 flushes, possibly with interspersed RCU barriers.

It's much simpler to count out the objects than to try and get the flushing right.

David

Tejun Heo

maintainer

2017-09-06 14:51 UTC #25 lore raw

Hello, David.

Quoted 17 lines · David Howells

With one of my latest patches to AFS, there's a set of cell records, where each cell has a manager work item that mainains that cell, including refreshing DNS records and excising expired records from the list. Performing the excision in the manager work item makes handling the fscache index cookie easier (you can't have two cookies attached to the same object), amongst other things.
There's also an overseer work item that maintains a single expiry timer for all the cells and queues the per-cell work items to do DNS updates and cell removal.
The reason that the overseer exists is that it makes it easier to do a put on a cell. The cell decrements the cell refcount and then wants to schedule the cell for destruction - but it's no longer permitted to touch the cell. I could use atomic_dec_and_lock(), but that's messy. It's cleaner just to set the timer on the overseer and leave it to that.

Quoted 4 lines

However, if someone does rmmod, I have to be able to clean everything up. The overseer timer may be queued or running; the overseer may be queued *and* running and may get queued again by the timer; and each cell's work item may be queued *and* running and may get queued again by the manager.

Thanks for the detailed explanation.

Quoted 6 lines

Quoted 1 line
Why can't it be done via the usual "flush from exit"?
Well, it can, but you need a flush for each separate level of dependencies, where one dependency will kick off another level of dependency during the cleanup.

Quoted 8 lines

So what I think I would have to do is set a flag to say that no one is allowed to set the timer now (this shouldn't happen outside of server or volume cache clearance), delete the timer synchronously, flush the work queue four times and then do an RCU barrier.
However, since I have volumes with dependencies on servers and cells, possibly with their own managers, I think I may need up to 12 flushes, possibly with interspersed RCU barriers.

Would it be possible to isolate work items for the cell in its own workqueue and use drain_workqueue()? Separating out flush domains is one of the main use cases for dedicated workqueues after all.

Quoted 2 lines

It's much simpler to count out the objects than to try and get the flushing right.

I still feel very reluctant to add generic counting & trigger mechanism to work items for this. I think it's too generic a solution for a very specific problem.

Thanks.

Signature · 1 line

tejun