Re: [PATCH v2 25/31] afs: Make {Y,}FS.FetchData an asynchronous operation

[David Howells <dhowells@redhat.com>]

I think this may need an additional bit (see attached).

David
---
afs: Fix hang due to FetchData RPC op being cancelled by signal

If a signal comes in just as an RPC operation is being queued to get a
channel for transmission, afs_make_call() will submit an immediate abort
and cancel the asynchronous work.  This is a problem for asynchronous
FetchData as the file-read routines don't get notified and don't therefore
get to inform netfslib, leaving netfslib hanging.

Fix this by:

 (1) Split the ->done() call op to have an ->immediate_cancel() op also
     that is called by afs_make_call() instead of ->done().

     It is undesirable from async FetchData's point of view to implement
     ->done() as this is also called from the received data processing
     loop, which is triggered by the async notification from AF_RXRPC.

 (2) Make the various async Probe RPCs use their ->immediate_cancel() go to
     the same handler as their ->done() call.

 (3) Don't provide the Lock RPCs, InlineBulkStatus RPC and YFS.RemoveFile2
     RPC with ->immediate_cancel() as their ->done() calls are only
     interested in looking at the response from the server.

 (4) Implement this for FetchData RPCs, making it schedule the async
     handler and wait for it so that it doesn't get cancelled.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/file.c      |    8 ++++++++
 fs/afs/fsclient.c  |    3 +++
 fs/afs/internal.h  |   17 +++++++++++++++++
 fs/afs/rxrpc.c     |   17 ++---------------
 fs/afs/vlclient.c  |    1 +
 fs/afs/yfsclient.c |    1 +
 6 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/afs/file.c b/fs/afs/file.c
index dbc108c6cae5..a2880fd3c460 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -314,6 +314,14 @@ void afs_fetch_data_async_rx(struct work_struct *work)
 	afs_put_call(call);
 }
 
+void afs_fetch_data_immediate_cancel(struct afs_call *call)
+{
+	afs_get_call(call, afs_call_trace_wake);
+	if (!queue_work(afs_async_calls, &call->async_work))
+		afs_deferred_put_call(call);
+	flush_work(&call->async_work);
+}
+
 /*
  * Fetch file data from the volume.
  */
diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 6380cdcfd4fc..1d9ecd5418d8 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -410,6 +410,7 @@ static const struct afs_call_type afs_RXFSFetchData = {
 	.op		= afs_FS_FetchData,
 	.async_rx	= afs_fetch_data_async_rx,
 	.deliver	= afs_deliver_fs_fetch_data,
+	.immediate_cancel = afs_fetch_data_immediate_cancel,
 	.destructor	= afs_flat_call_destructor,
 };
 
@@ -418,6 +419,7 @@ static const struct afs_call_type afs_RXFSFetchData64 = {
 	.op		= afs_FS_FetchData64,
 	.async_rx	= afs_fetch_data_async_rx,
 	.deliver	= afs_deliver_fs_fetch_data,
+	.immediate_cancel = afs_fetch_data_immediate_cancel,
 	.destructor	= afs_flat_call_destructor,
 };
 
@@ -1734,6 +1736,7 @@ static const struct afs_call_type afs_RXFSGetCapabilities = {
 	.op		= afs_FS_GetCapabilities,
 	.deliver	= afs_deliver_fs_get_capabilities,
 	.done		= afs_fileserver_probe_result,
+	.immediate_cancel = afs_fileserver_probe_result,
 	.destructor	= afs_fs_get_capabilities_destructor,
 };
 
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index b11b2dfb8380..2077f6c923e0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -210,6 +210,9 @@ struct afs_call_type {
 
 	/* Call done function (gets called immediately on success or failure) */
 	void (*done)(struct afs_call *call);
+
+	/* Handle a call being immediately cancelled. */
+	void (*immediate_cancel)(struct afs_call *call);
 };
 
 /*
@@ -1127,6 +1130,7 @@ extern void afs_put_wb_key(struct afs_wb_key *);
 extern int afs_open(struct inode *, struct file *);
 extern int afs_release(struct inode *, struct file *);
 void afs_fetch_data_async_rx(struct work_struct *work);
+void afs_fetch_data_immediate_cancel(struct afs_call *call);
 
 /*
  * flock.c
@@ -1362,6 +1366,19 @@ extern void afs_send_simple_reply(struct afs_call *, const void *, size_t);
 extern int afs_extract_data(struct afs_call *, bool);
 extern int afs_protocol_error(struct afs_call *, enum afs_eproto_cause);
 
+static inline struct afs_call *afs_get_call(struct afs_call *call,
+					    enum afs_call_trace why)
+{
+	int r;
+
+	__refcount_inc(&call->ref, &r);
+
+	trace_afs_call(call->debug_id, why, r + 1,
+		       atomic_read(&call->net->nr_outstanding_calls),
+		       __builtin_return_address(0));
+	return call;
+}
+
 static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
 				    gfp_t gfp)
 {
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 94fff4e214b0..066e5d70dabe 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -236,19 +236,6 @@ void afs_deferred_put_call(struct afs_call *call)
 		schedule_work(&call->free_work);
 }
 
-static struct afs_call *afs_get_call(struct afs_call *call,
-				     enum afs_call_trace why)
-{
-	int r;
-
-	__refcount_inc(&call->ref, &r);
-
-	trace_afs_call(call->debug_id, why, r + 1,
-		       atomic_read(&call->net->nr_outstanding_calls),
-		       __builtin_return_address(0));
-	return call;
-}
-
 /*
  * Queue the call for actual work.
  */
@@ -444,8 +431,8 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
 	call->error = ret;
 	trace_afs_call_done(call);
 error_kill_call:
-	if (call->type->done)
-		call->type->done(call);
+	if (call->type->immediate_cancel)
+		call->type->immediate_cancel(call);
 
 	/* We need to dispose of the extra ref we grabbed for an async call.
 	 * The call, however, might be queued on afs_async_calls and we need to
diff --git a/fs/afs/vlclient.c b/fs/afs/vlclient.c
index cac75f89b64a..adc617a82a86 100644
--- a/fs/afs/vlclient.c
+++ b/fs/afs/vlclient.c
@@ -370,6 +370,7 @@ static const struct afs_call_type afs_RXVLGetCapabilities = {
 	.name		= "VL.GetCapabilities",
 	.op		= afs_VL_GetCapabilities,
 	.deliver	= afs_deliver_vl_get_capabilities,
+	.immediate_cancel = afs_vlserver_probe_result,
 	.done		= afs_vlserver_probe_result,
 	.destructor	= afs_destroy_vl_get_capabilities,
 };
diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c
index 4e7d93ee5a08..f57c089f26ee 100644
--- a/fs/afs/yfsclient.c
+++ b/fs/afs/yfsclient.c
@@ -458,6 +458,7 @@ static const struct afs_call_type yfs_RXYFSFetchData64 = {
 	.op		= yfs_FS_FetchData64,
 	.async_rx	= afs_fetch_data_async_rx,
 	.deliver	= yfs_deliver_fs_fetch_data64,
+	.immediate_cancel = afs_fetch_data_immediate_cancel,
 	.destructor	= afs_flat_call_destructor,
 };