afs: Fix file locking

Fix the AFS file locking whereby the use of the big kernel lock (which
could be slept with) was replaced by a spinlock (which couldn't).  The
problem is that the AFS code was doing stuff inside the critical section
that might call schedule(), so this is a broken transformation.

Fix this by the following means:

 (1) Use a state machine with a proper state that can only be changed under
     the spinlock rather than using a collection of bit flags.

 (2) Cache the key used for the lock and the lock type in the afs_vnode
     struct so that the manager work function doesn't have to refer to a
     file_lock struct that's been dequeued.  This makes signal handling
     safer.

 (4) Move the unlock from afs_do_unlk() to afs_fl_release_private() which
     means that unlock is achieved in other circumstances too.

 (5) Unlock the file on the server before taking the next conflicting lock.

Also change:

 (1) Check the permits on a file before actually trying the lock.

 (2) fsync the file before effecting an explicit unlock operation.  We
     don't fsync if the lock is erased otherwise as we might not be in a
     context where we can actually do that.

Further fixes:

 (1) Fixed-fileserver address rotation is made to work.  It's only used by
     the locking functions, so couldn't be tested before.

Fixes: 72f98e72 ("locks: turn lock_flocks into a spinlock")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: jlayton@redhat.com

fs/afs/internal.h

@@ -430,6 +430,16 @@ struct afs_volume {
 	u8			name[AFS_MAXVOLNAME + 1]; /* NUL-padded volume name */
 };
 
+enum afs_lock_state {
+	AFS_VNODE_LOCK_NONE,		/* The vnode has no lock on the server */
+	AFS_VNODE_LOCK_WAITING_FOR_CB,	/* We're waiting for the server to break the callback */
+	AFS_VNODE_LOCK_SETTING,		/* We're asking the server for a lock */
+	AFS_VNODE_LOCK_GRANTED,		/* We have a lock on the server */
+	AFS_VNODE_LOCK_EXTENDING,	/* We're extending a lock on the server */
+	AFS_VNODE_LOCK_NEED_UNLOCK,	/* We need to unlock on the server */
+	AFS_VNODE_LOCK_UNLOCKING,	/* We're telling the server to unlock */
+};
+
 /*
  * AFS inode private data
  */
@@ -454,18 +464,16 @@ struct afs_vnode {
 #define AFS_VNODE_ZAP_DATA	3		/* set if vnode's data should be invalidated */
 #define AFS_VNODE_DELETED	4		/* set if vnode deleted on server */
 #define AFS_VNODE_MOUNTPOINT	5		/* set if vnode is a mountpoint symlink */
-#define AFS_VNODE_LOCKING	6		/* set if waiting for lock on vnode */
-#define AFS_VNODE_READLOCKED	7		/* set if vnode is read-locked on the server */
-#define AFS_VNODE_WRITELOCKED	8		/* set if vnode is write-locked on the server */
-#define AFS_VNODE_UNLOCKING	9		/* set if vnode is being unlocked on the server */
-#define AFS_VNODE_AUTOCELL	10		/* set if Vnode is an auto mount point */
-#define AFS_VNODE_PSEUDODIR	11		/* set if Vnode is a pseudo directory */
+#define AFS_VNODE_AUTOCELL	6		/* set if Vnode is an auto mount point */
+#define AFS_VNODE_PSEUDODIR	7 		/* set if Vnode is a pseudo directory */
 
 	struct list_head	wb_keys;	/* List of keys available for writeback */
 	struct list_head	pending_locks;	/* locks waiting to be granted */
 	struct list_head	granted_locks;	/* locks granted on this file */
 	struct delayed_work	lock_work;	/* work to be done in locking */
-	struct key		*unlock_key;	/* key to be used in unlocking */
+	struct key		*lock_key;	/* Key to be used in lock ops */
+	enum afs_lock_state	lock_state : 8;
+	afs_lock_type_t		lock_type : 8;
 
 	/* outstanding callback notification on this file */
 	struct afs_cb_interest	*cb_interest;	/* Server on which this resides */
@@ -843,6 +851,7 @@ extern void afs_clear_permits(struct afs_vnode *);
 extern void afs_cache_permit(struct afs_vnode *, struct key *, unsigned int);
 extern void afs_zap_permits(struct rcu_head *);
 extern struct key *afs_request_key(struct afs_cell *);
+extern int afs_check_permit(struct afs_vnode *, struct key *, afs_access_t *);
 extern int afs_permission(struct inode *, int);
 extern void __exit afs_clean_up_permit_cache(void);
 

fs/afs/rotate.c

@@ -46,8 +46,7 @@ bool afs_begin_vnode_operation(struct afs_fs_cursor *fc, struct afs_vnode *vnode
 		return false;
 	}
 
-	if (test_bit(AFS_VNODE_READLOCKED, &vnode->flags) ||
-	    test_bit(AFS_VNODE_WRITELOCKED, &vnode->flags))
+	if (vnode->lock_state != AFS_VNODE_LOCK_NONE)
 		fc->flags |= AFS_FS_CURSOR_CUR_ONLY;
 	return true;
 }
@@ -117,7 +116,7 @@ static void afs_busy(struct afs_volume *volume, u32 abort_code)
 	case VSALVAGING:	m = "being salvaged";	break;
 	default:		m = "busy";		break;
 	}
-	
+
 	pr_notice("kAFS: Volume %u '%s' is %s\n", volume->vid, volume->name, m);
 }
 
@@ -438,24 +437,67 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
 
 	_enter("");
 
-	if (!cbi) {
-		fc->ac.error = -ESTALE;
+	switch (fc->ac.error) {
+	case SHRT_MAX:
+		if (!cbi) {
+			fc->ac.error = -ESTALE;
+			fc->flags |= AFS_FS_CURSOR_STOP;
+			return false;
+		}
+
+		fc->cbi = afs_get_cb_interest(vnode->cb_interest);
+
+		read_lock(&cbi->server->fs_lock);
+		alist = rcu_dereference_protected(cbi->server->addresses,
+						  lockdep_is_held(&cbi->server->fs_lock));
+		afs_get_addrlist(alist);
+		read_unlock(&cbi->server->fs_lock);
+		if (!alist) {
+			fc->ac.error = -ESTALE;
+			fc->flags |= AFS_FS_CURSOR_STOP;
+			return false;
+		}
+
+		fc->ac.alist = alist;
+		fc->ac.addr  = NULL;
+		fc->ac.start = READ_ONCE(alist->index);
+		fc->ac.index = fc->ac.start;
+		fc->ac.error = 0;
+		fc->ac.begun = false;
+		goto iterate_address;
+
+	case 0:
+	default:
+		/* Success or local failure.  Stop. */
 		fc->flags |= AFS_FS_CURSOR_STOP;
+		_leave(" = f [okay/local %d]", fc->ac.error);
 		return false;
-	}
 
-	read_lock(&cbi->server->fs_lock);
-	alist = afs_get_addrlist(cbi->server->addresses);
-	read_unlock(&cbi->server->fs_lock);
-	if (!alist) {
-		fc->ac.error = -ESTALE;
+	case -ECONNABORTED:
 		fc->flags |= AFS_FS_CURSOR_STOP;
+		_leave(" = f [abort]");
 		return false;
+
+	case -ENETUNREACH:
+	case -EHOSTUNREACH:
+	case -ECONNREFUSED:
+	case -ETIMEDOUT:
+	case -ETIME:
+		_debug("no conn");
+		goto iterate_address;
 	}
 
-	fc->ac.alist = alist;
-	fc->ac.error = 0;
-	return true;
+iterate_address:
+	/* Iterate over the current server's address list to try and find an
+	 * address on which it will respond to us.
+	 */
+	if (afs_iterate_addresses(&fc->ac)) {
+		_leave(" = t");
+		return true;
+	}
+
+	afs_end_cursor(&fc->ac);
+	return false;
 }
 
 /*

fs/afs/security.c

@@ -284,8 +284,8 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
  * permitted to be accessed with this authorisation, and if so, what access it
  * is granted
  */
-static int afs_check_permit(struct afs_vnode *vnode, struct key *key,
-			    afs_access_t *_access)
+int afs_check_permit(struct afs_vnode *vnode, struct key *key,
+		     afs_access_t *_access)
 {
 	struct afs_permits *permits;
 	bool valid = false;

fs/afs/server_list.c

@@ -17,7 +17,7 @@ void afs_put_serverlist(struct afs_net *net, struct afs_server_list *slist)
 {
 	int i;
 
-	if (refcount_dec_and_test(&slist->usage)) {
+	if (slist && refcount_dec_and_test(&slist->usage)) {
 		for (i = 0; i < slist->nr_servers; i++) {
 			afs_put_cb_interest(net, slist->servers[i].cb_interest);
 			afs_put_server(net, slist->servers[i].server);